-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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(field): add zap.Dict to Field constructor list #1297
Conversation
This functionality doesn't require a new field type and can be implemented in the Zap package itself as a new ObjectMarshaler.
Codecov Report
@@ Coverage Diff @@
## master #1297 +/- ##
==========================================
+ Coverage 97.61% 97.77% +0.15%
==========================================
Files 52 52
Lines 3355 3366 +11
==========================================
+ Hits 3275 3291 +16
+ Misses 69 65 -4
+ Partials 11 10 -1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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, @hhk7734. This is a great idea!
I've made some changes to the implementation. Namely, instead of adding a new field type, I've implemented this functionality outside in terms of a new private ObjectMarshaler implementation. That way, the zapcore package doesn't see any changes (except new tests).
The change LGTM, but I'd like to get another maintainer to also look over it.
CC @mway @sywhang @prashantv
When I want to log a sub dictionary, it's hard to find that I can just log
zap.Object("k", ObjectMarshalerFunc(func(inner ObjectEncoder) error { ... }))
, and it seems unintuitive.So I added
zap.Dict
.Example