-
Notifications
You must be signed in to change notification settings - Fork 345
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 verbose flag for the run command #3193
Conversation
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.
We need to get rid off the fmt.Print
default favouring fmt.Fprintln(cmd.OutOrStdout(), ...)
. Even more, as it's a warning, I think we better use stderr via cmd.ErrOrStderr()
.
Additionally I'd wrap a function, ie, printOut(verbose bool)
where you have the if
condition, and call directly this function within the rest of the code. We will have a more readable code.
That sounds a nice idea. I'd push it further and make wrapped printing functions as methods for |
maybe worth switching to using klog like we do in the operator |
Sure. Alternatively we can finalize this and make a follow up issue to do it. |
030cd94
to
c37d1f3
Compare
Sounds good ! Applied the changes, thanks ! |
Thanks !
Release Note