-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
pkg/logflags: Display warning when unknown component for --log-output #1897
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.
LGTM
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.
Small changes, otherwise looks great, thanks!
pkg/logflags/logflags.go
Outdated
// If adding another value, do make sure to | ||
// update "Help about logging flags" in commands.go. | ||
default: | ||
if !unKnownHasPrint { // Just print first unknown component. |
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 we need to have this restriction, I don't mind printing a warning for each unknown log item.
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 try, it looks ok. I will change and rebase.
pkg/logflags/logflags.go
Outdated
// update "Help about logging flags" in commands.go. | ||
default: | ||
if !unKnownHasPrint { // Just print first unknown component. | ||
fmt.Fprintf(os.Stderr, "Warning: unknown %q for '--log-output', run 'dlv help log' for usage.\n", logcmd) |
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.
nit: Warning: unknown log output value %s for ...
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 remove for '--log-output'
that the same meaning as log output value
, avoid too long one line.
Warning: unknown log output value %q, run 'dlv help log' for usage.
Has done and rebase. The second version. |
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.
LGTM
No description provided.