Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update README and fix docker scripts #307

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

jhanca-robotecai
Copy link
Contributor

What does this PR do?

This PR fixes multiple small issues of the codebase on the development branch to get a working release with O3DE 2409.1. This PR targets the development branch, but the merge to main will follow instantly.

The changes include the following:

  • docker scripts changed to pull all repositories that are currently needed
  • docker scripts changed to pull stable release of the demo and all other branches instead of the development
  • default level is stored in the registry file instead of the obsolete autoexec (this includes docker scripts)
  • minor improvements to docker scripts
  • README files updated
  • release notes added

How was this PR tested?

Two docker images were built and tested using the following commands:

docker build -f Dockerfile.O3DE --build-arg ROSCON_DEMO_BRANCH=jh/changes --build-arg ROSCON_DEMO_COMMIT=b00db96 -t roscon2023_demo/o3de:latest .

docker build -f Dockerfile.ROS --build-arg ROSCON_DEMO_BRANCH=jh/changes --build-arg ROSCON_DEMO_COMMIT=b00db96 -t roscon2023_demo/ros:latest .

Copy link
Contributor

@arturkamieniecki arturkamieniecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall these changes look good, but i think that the docker files need a little more changes. Instead of inputting the tag name in the commit argument but as the branch argument. I think this would create less confusion for readers of the readme.

Docker/README.md Outdated Show resolved Hide resolved
@jhanca-robotecai
Copy link
Contributor Author

@arturkamieniecki
Thank you for the valid suggestion of changing branch/commit order in the docker and for applying this suggestion. Do you agree with the overall changes in this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants