Improve Docker build #239
Improve Docker build #239
Conversation
There was a problem hiding this comment.
I don't know much about docker, but the deletion of this seems that would break existing users:
COPYBARA_CONFIG=copy.bara.sky`
- allows you to specify a path to a config file, defaults to root
copy.bara.sky COPYBARA_SUBCOMMAND=migrate- allows you to change the command run, defaults to
migrate
- allows you to change the command run, defaults to
COPYBARA_OPTIONS=''- allows you to specify options for copybara, defaults to none
COPYBARA_WORKFLOW=default- allows you to specify the workflow to run, defaults to
default
- allows you to specify the workflow to run, defaults to
COPYBARA_SOURCEREF=''
|
If you want to handle environment arguments, this should definitely done by the executable itself. Adding this as a script to the container, feels like a big hack. Nevertheless, I added that functionality back to not break existing users for the time being. |
|
@mikelalcon PTAL |
| COPY --from=build /tmp/copybara/ /opt/copybara/ | ||
| RUN apt-get update && apt-get install -y \ | ||
| git \ | ||
| && rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
Add comment on why remove.
There was a problem hiding this comment.
IIUC this is the equivalent to "apt-get clean". Any reason for using this instead?
There was a problem hiding this comment.
This is suggested as best practice from Docker itself: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#apt-get
This is basically what is used in more than 99% of all Dockerfiles. Should I still add a comment?
There was a problem hiding this comment.
Yes, at least in the commit description. Can you: squash all changes in one and add a commit description that explains all we are doing here?
Thanks!
mering
force-pushed
the
docker-cmd
branch
2 times, most recently
from
ea3a306
to
582a1bf
Compare
This allows passing args to Copybara directly via CMD as an alternative to using environment variables.
Fixes #158
Fixes #235