-
Notifications
You must be signed in to change notification settings - Fork 612
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
feat: add --quiet flag to suppress verbose logs in #2998
base: master
Are you sure you want to change the base?
feat: add --quiet flag to suppress verbose logs in #2998
Conversation
Signed-off-by: olalekan odukoya <[email protected]>
2721762
to
04b3bdc
Compare
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.
Thanks
You're welcome! |
@@ -33,6 +33,7 @@ func newCopyCommand() *cobra.Command { | |||
} | |||
|
|||
copyCommand.Flags().BoolP("recursive", "r", false, "copy directories recursively") | |||
copyCommand.Flags().BoolP("quiet", "q", false, "suppress output logs") |
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 would consider making this quiet by default and adding -v --verbose for the verbose output, like the underling command we run and most commands.
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.
SGTM
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.
@AkihiroSuda should the PR be updated to go with this flow?
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 believe that's what he meant by "sounds good to me" (SGTM).
cmd/limactl/copy.go
Outdated
if quiet { | ||
sshCmd.Stdout = nil | ||
sshCmd.Stderr = nil | ||
} else { | ||
sshCmd.Stdout = cmd.OutOrStdout() | ||
sshCmd.Stderr = cmd.ErrOrStderr() | ||
} |
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.
Can be simplified:
if quiet { | |
sshCmd.Stdout = nil | |
sshCmd.Stderr = nil | |
} else { | |
sshCmd.Stdout = cmd.OutOrStdout() | |
sshCmd.Stderr = cmd.ErrOrStderr() | |
} | |
if !quiet { | |
sshCmd.Stdout = cmd.OutOrStdout() | |
sshCmd.Stderr = cmd.ErrOrStderr() | |
} |
The fields are nil
by default.
2431aa9
to
c97e496
Compare
Signed-off-by: olalekan odukoya <[email protected]>
Signed-off-by: olalekan odukoya <[email protected]>
c97e496
to
036c68a
Compare
debug, err := cmd.Flags().GetBool("debug") | ||
if err != nil { | ||
return err | ||
} | ||
if debug { | ||
|
||
if debug && !quiet { |
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 don't think the --quiet
option should override --debug
. It should only apply to the output of the ssh command itself.
Link to issue - #2963 (comment)