-
Notifications
You must be signed in to change notification settings - Fork 38
Add bash scripts for building and running docker from terminal #17
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
Conversation
Overall it looks good to me. Please address all the pending comments. If all my comments are addressed and @nicholaspalomo also approves I would be happy to move forward. |
docker/image_build.bash
Outdated
--tag "${IMAGE_TAG}" \ | ||
"${CONTEXT}" | ||
|
||
echo "Built image: ${IMAGE_TAG}" No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: In general, you should always leave a newline at the bottom of text-based files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@moavia90 could we still address this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manumerous newline at the end of bash script does not show in the view mode but is visible in edit mode of github
Thank you for addressing the review and adding this @moavia90! There are still two good comments from @nicholaspalomo that would be great if you can still address then. I already gave the approval so you will be able to merge right away afterwards. Can you use the "Squash and merge" functionality so we add everything into a single commit? In this case we do not need the ability to go back to the intermediate commits we added in this PR. Merci! |
Please go ahead and merge this. Thank you for the help! |
@manumerous I can't view "Merge pull request" button on my end that gives the option of "Squash and merge". |
Ok I will do it then. |
I have added the results related to memory usage across different PARALLEL_JOBS values. I’ve also included two bash scripts: - image_build.bash — for building the Docker image