-
Notifications
You must be signed in to change notification settings - Fork 55
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
Rename quickstart command [CLI-16] #102
Conversation
I believe auth0-cli/internal/cli/quickstart.go Line 44 in 0ee908b
auth0 quickstart to auth0 quickstarts .
|
Indeed, I changed it. Thanks! |
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.
🎊
|
||
return cmd | ||
} | ||
|
||
func quickstartDownloadCmd(cli *cli) *cobra.Command { | ||
func downloadQuickstart(cli *cli) *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.
(NAB) all these commands and subcommands init functions live on the same package (cli
), at some point it can be painful to find unique names, that's why I think keeping the "full subcommand path" as the func name is a good idea (quickstartsDownload()
in this case). Alternatively we can move each family of commands to its own package 🤷♂️
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 idea is to eventually have each family of commands on its own package, yes.
internal/cli/root.go
Outdated
@@ -59,7 +59,7 @@ func Execute() { | |||
// order of the comamnds here matters | |||
// so add new commands in a place that reflect its relevance or relation with other commands: | |||
rootCmd.AddCommand(loginCmd(cli)) | |||
rootCmd.AddCommand(quickstartCmd(cli)) | |||
rootCmd.AddCommand(quickstartsCmd(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.
minor/NAB: I wonder if the intuitive flow would be: login -> create an app -> quickstart...
in that case let's switch the order here (since it is reflected on the overall CLI help, usage text, etc)
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.
Yes, you need a client id to be able to run a QS app, so you need to create an app first. I've switched the order in 061d3e4.
Thanks! |
Description
This PR renames the
quickstart
command to its plural form,quickstarts
.Testing
Checklist
master