-
-
Notifications
You must be signed in to change notification settings - Fork 221
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
Add add_log_level() and add_logger_name() stdlib processors #44
Conversation
@hynek please review |
Thanks! Couple very minor issues:
Thanks again! |
@hynek take 2 |
The log level is added to the event dict. | ||
""" | ||
event_dict = add_log_level(None, 'error', {}) | ||
assert 'level' in event_dict |
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.
oops sorry i overlooked it in the first round: this is implicitly tested in the next line, let’s keep it simple
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've removed it. Though it will fail with a KeyError
instead of an AssertionError
if the test fails. But I agree, simpler is better. (Hence my combined "alias" test, btw.)
one more quick thing: I thing the changelog should refer the PR that actually got merged (i.e. 44) not a related issue. Thanks again, will merge afterwards! |
Thanks! |
Add add_log_level() and add_logger_name() stdlib processors
See #43.