-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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(cmd/influxd): parse log-level CLI opts directly to correct type #20196
Conversation
"go.uber.org/zap/zapcore" | ||
) | ||
|
||
type levelValue zapcore.Level |
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.
This boilerplate file is pretty much identical to kit/cli/idflag.go
, but with zapcore.Level
instead of influxdb.ID
.
} | ||
if s := v.GetString(envVar); s != "" { | ||
_ = (*destP).Set(v.GetString(envVar)) | ||
} |
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.
This block is mostly boilerplate, following the same pattern as the preceding case
s
"go.uber.org/zap/zapcore" | ||
) | ||
|
||
type levelValue zapcore.Level |
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.
Is this definition just a way to declare methods on the zapcore.Level type? Nice trick!
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 think) yes, specifically so we can redefine methods like String
and Set
that already exist on the zapcore.Level type.
Part of #19976
This took more boilerplate than I expected, I could be convinced it's not worth the trouble.