-
Notifications
You must be signed in to change notification settings - Fork 694
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
Changing docker client config attributes to Bazel labels #2042
Conversation
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.
The PR and logic itself seems sound to me, I don't see any use-cases where an absolute path would be desirable. Despite this, it should still possible for the toolchain to use the client config specified in the home directory, or with the DOCKER_CONFIG
environment variable.|
Pending response to the comments I left, I'm happy to approve 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.
Thank you for making those changes! LGTM
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #2040
What is the new behavior?
toolchain_configure
only takes a Bazel labelDoes this PR introduce a breaking change?
Existing applications will have to change the
client_config
of theirtoolchain_configure
rule to be a Bazel label.Other information
This is more correct than #2032 and #2041, but it introduces a breaking change.