-
Notifications
You must be signed in to change notification settings - Fork 6
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
[enhancement] add couple missing zapcore types #18
Conversation
56b6fa7
to
1628a0b
Compare
Converted to draft since ArrayMarshalerType seems to not work yet |
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.
When we do type conversion, should we make some assertion on the type to prevent the application from panicking?
e.g.
var myVar interface{}
myVar = "hello world"
myString := myVar.(string)
// this will panic when myVar couldn't be converted to string
var myVar interface{}
myVar = "hello world"
myString, ok := myVar.(string)
if (!ok) {
// not panic
}
All of the direct type conventions are the same as the once that zapcore does, so i don't believe they would require assertions. |
should be fine if thats the case |
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
I will add a small test tomorrow |
@natebwangsut its ready from my side, tested the workflow with |
@natebwangsut friendly bump |
Thanks for contribution @mikhail5555 ! |
Add convertion to the following fields: