-
Notifications
You must be signed in to change notification settings - Fork 2k
extract config into a dedicated module #6210
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
22f2fd5
to
b6d7698
Compare
Signed-off-by: Nicolas De Loof <[email protected]>
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'm honestly hesitant about this change. It is already difficult to update / change anything related to the CLI config due to its broad usage internally and externally to Docker.
I'm of the opinion that we should rather be decoupling third parties from the config by providing an alternative instead of officially supporting an integration with the existing config.
Any changes made to the CLI config would now also have to be made to an exported package, which would make this even more difficult to change/remove in future.
Sure, but they already do. Not being a separate go module obviously didn't prevented this usage
that's still an option, I don't see how this PR would prevent such a move. If you want third-party to access cli/config with a dedicated "officially supported" package, you'll have to somehow deprecate code in docker/cli so they stop relying on this legacy |
I need to have a closer look at this one. These packages need sorting out altogether as there's parts that should be considered "exclusive" to the CLI itself (after all, the config-file was the config file for the CLI), and parts that became an implicit "API" (most notably the auth-config, which was embedded in the CLI config). Separate from that, these packages have a MUCH too broad scope. For the auth-config part, I've contemplated having a separate package (which could be for a "plain-text" credentials helper) that's embedded in the config, so that it can be used separate from the other parts of teh config. But altogether, many parts are currently in an intermediate state, and moving it to a module as-is right now means we'll have to maintain those things forever (SemVer and such); at at least, we'd have to alias the module in the old location, otherwise we end up with issues if (indirect) dependencies expect the old location, which will break. That's not to say that there's no problem to be fixed, but some work is needed before we can make this happen; in the meantime, it might be better to keep the status quo. |
What I did
Extracted the
config
package into a dedicated (sub)module, so third-party component which need to interact with CLI config can use this codebase without inheriting the whole docker/cli dependenciesHow I did it
moved code to a at top-level folder and created go.mod
How to verify it
Human readable description for the release notes
introduce github.com/docker/cli/config as an isolated go module for third-party software to access docker CLI configuration files
A picture of a cute animal (not mandatory but encouraged)