-
Notifications
You must be signed in to change notification settings - Fork 98
add hidden 'get join-command' command for embedded-cluster #5280
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
add hidden 'get join-command' command for embedded-cluster #5280
Conversation
check error codes, test error code handling in both requests
606f44c
to
cba3dab
Compare
cmd/kots/cli/get-joincommand.go
Outdated
if err != nil { | ||
return err | ||
} | ||
fmt.Println(joinCmd) |
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 think we should also support a -ojson
output as people will want to use this command mostly in ci
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.
done
cmd/kots/cli/get-joincommand.go
Outdated
}, | ||
} | ||
|
||
cmd.Flags().StringVarP(&format, "output", "o", "string", "Output format. One of: string, json") |
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.
sorry, but to be consistent with other commands, we should use ""
instead of string
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.
okay - so ""
or string
gets string output, json
gets json output?
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.
no need for string
at all IMO
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 again
What this PR does / why we need it:
Which issue(s) this PR fixes:
Does this PR require a test?
Does this PR require a release note?
Does this PR require documentation?