chore: version sub command remove --version and -v flag
#2090
Conversation
cli/root.go
Outdated
| func versionCmd() *cobra.Command { | ||
| return &cobra.Command{ | ||
| Use: "version", | ||
| Short: "Version for coder", |
There was a problem hiding this comment.
Suggestion: I think this reads a bit better, but up to you to include or not.
| Short: "Version for coder", | |
| Short: "Show coder version", |
cli/root.go
Outdated
| cmd.Flags().BoolP("placeholder", "v", false, "This flag is a placeholder to make the cobra version flag work appropriately") | ||
| _ = cmd.Flags().MarkHidden("placeholder") |
There was a problem hiding this comment.
Cobra doesn't add this flag if Version isn't specified in the root. I'd propose that being a cleaner alternative.
Having --version and coder version seems unnecessary to me. I'd prefer to have coder version.
There was a problem hiding this comment.
I’m fine with only having version but having both is consistent with help (or do I misremember?) and I think covers a common use case. I.e. there’s a 50/50 chance someone will blindly do either --version or version instead of checking help 😄.
There was a problem hiding this comment.
I like how Go just does go version. I haven't found it to be confusing tbh
There was a problem hiding this comment.
Correct cobra does not. However if we want to support --version, we would need a persistent pre-run function to support the flag. Cobra has some first class support for --version that works even if there are extra args, eg: coder --version workspaces.
I think it is reasonable to use their first class support for this reason.
As for --version or coder version. I don't see anything wrong with supporting both. It's nice as a user to do w/e is more familiar to you.
Maybe coder version can report more info like the version of the deployment you are logged into.
Examples:
git version&git --versiondocker --versionis less verbose thandocker versiondocker-compose --versionis less verbose thandocker-compose version
There was a problem hiding this comment.
It's fine to support both, I don't really understand why we need to though. Seems like supporting coder version is fine, and supporting both seems to not really satisfy a need, but adds an additional user-contract for us to test.
There was a problem hiding this comment.
I'll drop the flag for now then and just do coder version
version sub command and reserve -v flag on root.version sub command remove --version and -v flag
What this does (#1543)
Removes
coder -vandcoder --version.Now do
coder versionto see coder version.