-
Notifications
You must be signed in to change notification settings - Fork 42
Conversation
@tedteng in description |
@neo-liang-sap seems you are just close the issue #259. |
/lgtm |
pkg/cmd/root.go
Outdated
@@ -129,6 +130,7 @@ func init() { | |||
|
|||
RootCmd.PersistentFlags().BoolVarP(&cachevar, "no-cache", "c", false, "no caching") | |||
RootCmd.PersistentFlags().StringVarP(&outputFormat, "output", "o", "yaml", "output format yaml or json") | |||
RootCmd.PersistentFlags().BoolVarP(&debugSwitch, "debug", "d", false, "enable debug level 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.
RootCmd.PersistentFlags().BoolVarP(&debugSwitch, "debug", "d", false, "enable debug level output") | |
RootCmd.PersistentFlags().BoolVarP(&debugSwitch, "debug", "d", false, "enable debug level output") |
Could we name it "verbose" as it is not necessarily for debugging?
Just a personal preference though.
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.
Mhm I see that there is already a previous commit that introduces the debug
flag.
Maybe @DockToFuture has an opinion about the naming of the flag aswell?
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.
sure, np, that part I just copy from the previous commit for my testing purpose at that time, before that commit merger. Now it changes back.
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.
So you want to have the flag named --debug
and not --verbose
?
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 mean after rebase it change back. I am fine with verbose
I can change --debug
to --verbose
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. @danielfoehrKn let me know if there anything else need update.
pkg/cmd/ssh_aws.go
Outdated
@@ -51,6 +51,8 @@ type AwsInstanceAttribute struct { | |||
MyPublicIP string | |||
} | |||
|
|||
var sshCmd 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.
could be moved to line 84?
This PR seems to include also changes from this previous commit - could you rebase? |
done |
@danielfoehrKn do you still have comments regarding this PR? |
@danielfoehrKn ping, do you still have comments on this PR? |
What this PR does / why we need it:
enable ssh debug msg
enhance error msg debug msg, make more meaning
Which issue(s) this PR fixes:
Fixes #265
Special notes for your reviewer:
Release note: