-
-
Notifications
You must be signed in to change notification settings - Fork 147
Conversation
GitHub is claiming that this doesn't merge cleanly, can you rebase on master? |
I think I fixed it. Thanks! |
Level Severity | ||
Client *Client | ||
Level Severity | ||
LoggerName string |
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.
Hmm, should we call this Logger
to match the field in Packet
?
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.
We could: I thought it was a little confusing to name it that, but if you'd prefer it match I'd be on board. Maybe I could add a comment.
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.
Yeah, add a comment and call it Logger
.
The tests also need fixing: https://travis-ci.org/cupcake/raven-go/jobs/16118609 |
Yeah, I saw that: I think it's fixed now. Field too! |
A check should be added to |
Okay, I actually looked at the tests this time. 😳 😉 |
Awesome, squash and I'll merge. |
`logger` is a required field (although a default is specified). <http://sentry.readthedocs.org/en/latest/developer/client/index.html#bui lding-the-json-packet> If you want to use a Writer with the log pkg, you can specify the logger name there too. Update client-test for required logger field Change LoggerName to Logger to match Packet field Sloppy! Maybe I'm serious about tests this time
👢 |
Thanks! |
Add special sentry field support.
logger
is a required field (although a default is specified).http://sentry.readthedocs.org/en/latest/developer/client/index.html#building-the-json-packet
If you want to use a
Writer
with thelog
pkg, you can specify the logger name there too.