-
Notifications
You must be signed in to change notification settings - Fork 171
Use Docker Desktop FF for the profiles feature flag #280
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
Conversation
|
|
||
| // featureListCommand creates the `feature list` command | ||
| func featureListCommand(dockerCli command.Cli) *cobra.Command { | ||
| func featureListCommand(dockerCli command.Cli, features features.Features) *cobra.Command { |
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.
We shouldn't be able to enable/disable the profiles feature when running in DockerDesktop but we shouldn't we be able to list it?
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.
Hmm, good question. Our current experience would make that a little awkward I think. If we listed it but then when they tried to enable it we said "we don't know what that flag is," it might seem like a bug. We could probably be smarter about it and say "only supported in Docker CE or container mode".
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.
Since profiles are supported in both CE an Desktop, CE users will still toggle profile support via the cli?
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.
Correct, they'll still use the cli toggle. DD users will have to use the DD feature flag.
| addQuietFlag(flags, &opts.Quiet) | ||
| if isWorkingSetsFeatureEnabled(dockerCli) { | ||
| if features.IsProfilesFeatureEnabled() { | ||
| addWorkingSetFlag(flags, &opts.WorkingSet) |
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.
This is still controversial, right?
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 not sure I understand, could you elaborate?
What I did
Instead of checking the "profiles" feature to enable profiles, it will check the DD feature flag. This will enable us to roll out the feature in sync with the DD UI changes.
This only applies when the plugin is running with Docker Desktop installed. For Docker CE and container mode, this will still use the "profiles" CLI flag.
Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did