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

Add build GH action to demos repo #35

Closed
mkhansenbot opened this issue Aug 28, 2024 · 5 comments · Fixed by #37
Closed

Add build GH action to demos repo #35

mkhansenbot opened this issue Aug 28, 2024 · 5 comments · Fixed by #37
Assignees

Comments

@mkhansenbot
Copy link
Contributor

I'm planning to put a build GH action on this (demos) repo, to ensure all PRs can build (we need this for the competition). I'd like to propose that we require all demos to contain a Dockerfile or docker-compose.yaml and a build.sh . Then I can have the CI do a find . -name build.sh and iterate through all the build scripts found. That would work with the old demos as well as the new submissions.

@franklinselva
Copy link

I would like to see the purpose of the build.sh script for each sub directory in detail. Can you please share what are the actions that will be performed from Github Action? Are you recommending to use build.sh as an build command to be run on top of docker image?

For instance, one of the demo's build script will also make sure all the pre-requisties are installed before building and running the demos. I understand this changes the purpose of build.sh and handles the overall demo related actions to build, run and clean the builds. Should we go for a dedicated shell function specific for Github Actions? or Break the script to build.sh and run.sh?

To give more insight on the current usage of the shell script, I am adding the documentation here.

@mkhansenbot
Copy link
Contributor Author

mkhansenbot commented Aug 30, 2024

We currently have both a build.sh and a run.sh for the demos in the docker repo, which will be moved here. For now, I'm only planning to have the action do the build. Ideally, each demo should also include a run.sh and a test.sh or something similar that allows us to test the demos (headlessly) through the CI without hard-coding any commands into the CI.

@franklinselva
Copy link

franklinselva commented Aug 30, 2024

Thanks for the pointer related to the shell scripts. I almost missed it.

We currently have both a build.sh and a run.sh for the demos in the docker repo, which will be moved here. For now, I'm only planning to have the action do the build. Ideally, each demo should also include a run.sh and a test.sh or something similar that allows us to test the demos (headlessly) through the CI without hard-coding any commands into the CI.

I can see we are planning for a very sophisticated validation pipeline for the demos. Do we also plan to use a CI provider different from Github, like Self Hosted runners?

As the first stage of such pipeline, will build.sh have only the docker image build? This will still contradict what is expected from the documentation. I feel adding a test.sh / ci.sh specifically for the GitHub actions would be a better fit. This shell script should build all the ROS packages and also build the docker image related to the demos to make the CI pass. If the idea seems feasible, I can already see we don't need separate script for GitHub actions. A common script traversing through each subdirectory should take up the job.

@mkhansenbot
Copy link
Contributor Author

The initial intent of the build.sh was to simplify building the demos for the user. By using build.sh we ensure that the script works.

@mkhansenbot
Copy link
Contributor Author

Fixed in #37

@Bckempa Bckempa added this to the jazzy-2024.10.0 milestone Sep 5, 2024
@Bckempa Bckempa linked a pull request Sep 5, 2024 that will close this issue
RBinsonB pushed a commit to RBinsonB/demos that referenced this issue Sep 6, 2024
RBinsonB pushed a commit to RBinsonB/demos that referenced this issue Sep 6, 2024
mazm0002 pushed a commit to element-robotics/demos that referenced this issue Sep 9, 2024
mazm0002 pushed a commit to element-robotics/demos that referenced this issue Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants