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

Support exception method in loggers #52

Merged
merged 3 commits into from Mar 16, 2015

Conversation

leplatrem
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d870ce0 on leplatrem:support_exception_printlogger into d6dfd8f on hynek:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d870ce0 on leplatrem:support_exception_printlogger into d6dfd8f on hynek:master.

@hynek
Copy link
Owner

hynek commented Mar 14, 2015

Thanks!

  1. every new test requires a doc string like https://plus.google.com/+JonathanLange/posts/YA3ThKWhSAj
  2. I think we should somehow make sure, that no method is missing. How about using the keys of _NAME_TO_LEVEL sans notset?
  3. Please use py.test’s parametrize instead of for loops: http://pytest.org/latest/parametrize.html It makes nicer error reports.
  4. I forgot about the changelog again. Could you add an entry like “Better log.exception() within structlog.” and point it to both pull requests?

Thanks again!

@leplatrem
Copy link
Contributor Author

I'll update the PR, no problem.

Updating the changelog in the PR does not make it easier for the maintainer (and the contributor!), since it will often trivial conflicts.

IHMO updating it in the merge commit is a better approach :)

@leplatrem leplatrem force-pushed the support_exception_printlogger branch from edaa92f to ad3973f Compare March 16, 2015 15:34
@hynek
Copy link
Owner

hynek commented Mar 16, 2015

speaking of which, could you rebase please :)

@leplatrem
Copy link
Contributor Author

Done!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ad3973f on leplatrem:support_exception_printlogger into 0a90029 on hynek:master.

@hynek
Copy link
Owner

hynek commented Mar 16, 2015

Thanks!

hynek added a commit that referenced this pull request Mar 16, 2015
@hynek hynek merged commit 031aa1d into hynek:master Mar 16, 2015
@zmsmith zmsmith mentioned this pull request Jun 9, 2015
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

3 participants