-
Notifications
You must be signed in to change notification settings - Fork 104
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
Refactor/Cleanup for output parameter #1645
Conversation
Use type for output Add separate Options for plan status Add tests to validate invalid output parameter Cleanup tests to use separate t.Run executions Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
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 have just few nits otherwise LGTM :)
pkg/kudoctl/cmd/init.go
Outdated
@@ -110,7 +110,7 @@ func newInitCmd(fs afero.Fs, out io.Writer, errOut io.Writer, client *kube.Clien | |||
f.StringVarP(&i.image, "kudo-image", "i", "", "Override KUDO controller image and/or version") | |||
f.StringVarP(&i.imagePullPolicy, "kudo-image-pull-policy", "", "Always", "Override KUDO controller image pull policy") | |||
f.StringVarP(&i.version, "version", "", "", "Override KUDO controller version of the KUDO image") | |||
f.StringVarP(&i.output, "output", "o", "", "Output format") | |||
f.StringVarP((*string)(&i.output), "output", "o", "", "Output format") |
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.
could we do rather something like i.output.AsString? 🤔 to not hardcode here expectations about the type alias...
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 can do:
f.StringVarP(i.output.AsStringPtr, "output", "o", "", "Output format")
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.
What Alena said, but otherwise 🚢
Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Use type for output
Add separate Options for plan status
Add tests to validate invalid output parameter
Cleanup tests to use separate t.Run executions
Signed-off-by: Andreas Neumann aneumann@mesosphere.com
What this PR does / why we need it:
Started to add
--output yaml
topackage verify
andpackage create
and ended up refactoring cleaning up a couple things - Decided to make it a separate PR for easier Review