-
Notifications
You must be signed in to change notification settings - Fork 548
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
crane: add catalog argument use annotation #1473
Conversation
also considered using the same language/syntax as auth get. ie It does feel required rather than optional - but recommended by cobra doesn't mean that this project doesn't have other preferred practices. |
cmd/crane/cmd/catalog.go
Outdated
@@ -24,7 +24,7 @@ import ( | |||
// NewCmdCatalog creates a new cobra.Command for the repos subcommand. | |||
func NewCmdCatalog(options *[]crane.Option) *cobra.Command { | |||
return &cobra.Command{ | |||
Use: "catalog", | |||
Use: "catalog {REGISTRY}", |
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.
Use: "catalog {REGISTRY}", | |
Use: "catalog [REGISTRY]", |
Yeah let's use the same form we use elsewhere.
It might also help to add a usage section with some examples, so folks know what kinds of values to expect [REGISTRY]
to mean.
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.
Updated to square brackets & included a standard example using some existing patterns.
crane catalog --help
List the repos in a registry
Usage:
crane catalog [REGISTRY] [flags]
Examples:
# list the repos for reg.example.com
$ echo "reg.example.com" | crane catalog
# or
$ crane catalog reg.example.com
Flags:
-h, --help help for catalog
Global Flags:
--allow-nondistributable-artifacts Allow pushing non-distributable (foreign) layers
--insecure Allow image references to be fetched without TLS
--platform platform Specifies the platform in the form os/arch[/variant][:osversion] (e.g. linux/amd64). (default all)
-v, --verbose Enable debug logs
Co-authored-by: Jason Hall <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1473 +/- ##
=======================================
Coverage 73.18% 73.18%
=======================================
Files 115 115
Lines 8809 8809
=======================================
Hits 6447 6447
Misses 1712 1712
Partials 650 650 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
lolwut How does this work for |
Apologies in advance:
|
Are there any additional steps that I need to make prior to this being clear for merge? |
Following Cobra Recommended Syntax for "Use", I added the required argument annotation to the command.
While the registry argument may be assumed, I believe this improves the user experience by explicitly defining required arguments. Runtime without a registry
Error: accepts 1 arg(s), received 0
and use of the--help
flag does not define what the expected argument should be.