Skip to content
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

Option to disable/rename verbosity field #14

Merged
merged 1 commit into from Jun 10, 2022
Merged

Option to disable/rename verbosity field #14

merged 1 commit into from Jun 10, 2022

Conversation

hn8
Copy link
Contributor

@hn8 hn8 commented Feb 7, 2022

Simpler alternative for #13

zerologr.go Outdated
@@ -33,10 +32,11 @@ var (
NameFieldName = "logger"
// NameSeparator separates names for logr.WithName
NameSeparator = "/"
// VerbosityFieldName is the field key for logr.Info verbosity
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document that "" means not to emit that value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thockin Fixed

@thockin thockin self-assigned this Feb 7, 2022
@hn8 hn8 force-pushed the verbosity-key branch 2 times, most recently from 435abc0 to cc7a7b4 Compare February 8, 2022 01:26
@tonglil
Copy link

tonglil commented Feb 10, 2022

Looks good to me

Just wondering if you are ok with having duplicate/conflicting verbosity/leveling in the output without an option to turn it off? ie: level=info and v=0

@hn8
Copy link
Contributor Author

hn8 commented Feb 11, 2022

@tonglil Both zerolog.LevelFieldName and new zerologr.VerbosityFieldName are configurable. They may be redundant but always consistent on verbosity. zerolgor does not change zerolog global variable anymore.

@tonglil
Copy link

tonglil commented Feb 11, 2022

Thanks, forgot about zerolog.LevelFieldName.

@tonglil
Copy link

tonglil commented Mar 15, 2022

Any way to merge this please? Happy to help be a contributor.

@tonglil
Copy link

tonglil commented May 24, 2022

Is anyone able to take a look here? Thanks!

@@ -16,8 +16,7 @@
// Levels in logr correspond to custom debug levels in Zerolog. Any given level
// in logr is represents by `zerologLevel = 1 - logrLevel`.
// For example V(2) is equivalent to Zerolog's TraceLevel, while V(1) is
// equivalent to Zerolog's DebugLevel. Zerolog's usual "level" field is
// disabled globally and replaced with "v", whose value is a number and is only
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of "Zerolog's usual "level" field is disabled globally" implies that it is no longer disabled. But I don't see such a change in this PR. Was that already done earlier, without updating this doc comment?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like #7 did that, so this doc update is just a follow-up.

Copy link
Collaborator

@pohly pohly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'll give Tim a bit more time as he had been reviewing it before and then will merge it.

@pohly pohly merged commit 8ee7818 into master Jun 10, 2022
@hn8 hn8 deleted the verbosity-key branch June 13, 2022 09:35
@tonglil
Copy link

tonglil commented Jun 14, 2022

Thanks for the merge @pohly, any chance a release could be made?

@hn8
Copy link
Contributor Author

hn8 commented Jun 14, 2022

@tonglil released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants