Name the final stage explicitly, move scripts, other minor refactoring#91
Merged
Name the final stage explicitly, move scripts, other minor refactoring#91
Conversation
victorlin
changed the title
tsibley
reviewed
Oct 13, 2022
victorlin
force-pushed
the
victorlin/refactor
branch
4 times, most recently
from
51e6c92 to
ae92599
Compare
tsibley
reviewed
Oct 13, 2022
tsibley
approved these changes
Oct 13, 2022
Contributor
tsibley
left a comment
tsibley
left a comment
There was a problem hiding this comment.
My review comments here are minor, so merge when you feel this is ready.
For (1) consistency and (2) defense against any future meta-chars in substituted variable values. Remove quotes around strings without parameter expansions
Without a name, this stage is currently named `stage-1` in the logs and is the `docker build` target default. Add a name to make things more self-descriptive. This also allows us to simplify the variable name from BASE_BUILDER_IMAGE to BUILDER_IMAGE.
The builder and final image are referenced in different orders, which is a bit inconsistent. Make it consistent by always referencing builder before final, to mirror the sequential order in the Dockerfile.
Previously, there were two distinct types of scripts in devel/: 1. Scripts that interface with the Docker CLI (build/push/tag) 2. Scripts used in the Dockerfile's builder stage Intents and interactions are more clear if these scripts live in separate folders. Choosing to keep (1) in devel/ and move (2) to a new folder builder-scripts/.
victorlin
force-pushed
the
victorlin/refactor
branch
from
ae92599 to
3d45a61
Compare
Member
Author
|
Will merge once CI passes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
See individual commits.
Related issue(s)
N/A
Testing