-
Notifications
You must be signed in to change notification settings - Fork 2k
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
move cli-plugins metadata types/consts to a separate package #5902
Conversation
This prevents cli-plugins having to import the plugin-manager. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
48b4e5c
to
292713c
Compare
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (40.62%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #5902 +/- ##
==========================================
+ Coverage 59.26% 59.29% +0.02%
==========================================
Files 357 357
Lines 29771 29771
==========================================
+ Hits 17645 17653 +8
+ Misses 11153 11144 -9
- Partials 973 974 +1 🚀 New features to boost your workflow:
|
"github.com/docker/cli/cli-plugins/manager" | ||
"github.com/docker/cli/cli-plugins/metadata" |
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.
Went a bit back-and-forth what made most sense;
"github.com/docker/cli/cli-plugins/plugin/metadata"
"github.com/docker/cli/cli-plugins/metadata"
In the end, decided on the last one (for now), because it's shared between both, but let me know what you think!
const ( | ||
// CommandAnnotationPlugin is added to every stub command added by | ||
// AddPluginCommandStubs with the value "true" and so can be | ||
// used to distinguish plugin stubs from regular commands. | ||
CommandAnnotationPlugin = "com.docker.cli.plugin" | ||
|
||
// CommandAnnotationPluginVendor is added to every stub command | ||
// added by AddPluginCommandStubs and contains the vendor of | ||
// that plugin. | ||
CommandAnnotationPluginVendor = "com.docker.cli.plugin.vendor" | ||
|
||
// CommandAnnotationPluginVersion is added to every stub command | ||
// added by AddPluginCommandStubs and contains the version of | ||
// that plugin. | ||
CommandAnnotationPluginVersion = "com.docker.cli.plugin.version" | ||
|
||
// CommandAnnotationPluginInvalid is added to any stub command | ||
// added by AddPluginCommandStubs for an invalid command (that | ||
// is, one which failed it's candidate test) and contains the | ||
// reason for the failure. | ||
CommandAnnotationPluginInvalid = "com.docker.cli.plugin-invalid" | ||
|
||
// CommandAnnotationPluginCommandPath is added to overwrite the | ||
// command path for a plugin invocation. | ||
CommandAnnotationPluginCommandPath = "com.docker.cli.plugin.command_path" | ||
) |
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.
Didn't mark these as deprecated yet (nor did I remove the GoDoc strings); we could if we wanted to though.
const ( | ||
// CommandAnnotationPlugin is added to every stub command added by | ||
// AddPluginCommandStubs with the value "true" and so can be | ||
// used to distinguish plugin stubs from regular commands. | ||
CommandAnnotationPlugin = "com.docker.cli.plugin" | ||
|
||
// CommandAnnotationPluginVendor is added to every stub command | ||
// added by AddPluginCommandStubs and contains the vendor of | ||
// that plugin. | ||
CommandAnnotationPluginVendor = "com.docker.cli.plugin.vendor" | ||
|
||
// CommandAnnotationPluginVersion is added to every stub command | ||
// added by AddPluginCommandStubs and contains the version of | ||
// that plugin. | ||
CommandAnnotationPluginVersion = "com.docker.cli.plugin.version" | ||
|
||
// CommandAnnotationPluginInvalid is added to any stub command | ||
// added by AddPluginCommandStubs for an invalid command (that | ||
// is, one which failed it's candidate test) and contains the | ||
// reason for the failure. | ||
CommandAnnotationPluginInvalid = "com.docker.cli.plugin-invalid" | ||
|
||
// CommandAnnotationPluginCommandPath is added to overwrite the | ||
// command path for a plugin invocation. | ||
CommandAnnotationPluginCommandPath = "com.docker.cli.plugin.command_path" | ||
) | ||
|
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.
Are these used by other plugins? Should we deprecate and alias before removing?
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.
Oh! There are aliases, but moved to a separate file; see further up; cli-plugins/manager/annotations.go
I think I had that move in a separate commit originally 🤔 (I can do so separately to make it more clear probably)
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.
Ahh right, I confused it with the new file 🙈
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 to double-check myself, but it was there 🤗
move cli-plugins metadata types/consts to a separate package
This prevents cli-plugins having to import the plugin-manager.
move cli-plugins annotation consts to a separate package
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)