-
Notifications
You must be signed in to change notification settings - Fork 24
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 support for additional builder arguments #17
Conversation
Heya, thanks for the PR! This looks sensible. Thoughts on making this an array like the Docker Compose plugin? |
I certainly wish that all the plugins would be similar enough (if not identical). The ecr-cache plugin uses the name "additional build args" as a plain string which I think is slightly better than when they use the "args" name in the docker plugin. In truth, args/build_args/additional_build_args needs to be cleared up before we press on. But that's a nit among gnats!
So I propose (if you accept): We move ahead with this PR (post merge-conflict) while also opening issues in both this repo and the ecr-cache repo to make them both accept arrays and strings [as well as perhaps identify what the best course of naming for these parameters would be --optional]. |
This PR has been updated to avoid a conflict with v1.3.0 and retargets v1.4.0 |
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.
I had another think about this one, let's just leave it as a string. Thanks again for the PR, I'll get this out after the target
fix.
Co-authored-by: Ryan Ling <ryan@outlook.com.au>
Co-authored-by: Ryan Ling <ryan@outlook.com.au>
This addresses my comments in #15