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

Make ProcessorFormatter able to log dict #189

Merged
merged 1 commit into from Feb 2, 2019
Merged

Make ProcessorFormatter able to log dict #189

merged 1 commit into from Feb 2, 2019

Conversation

@ZlatyChlapec
Copy link
Contributor

@ZlatyChlapec ZlatyChlapec commented Nov 30, 2018

Fixes error when logging dict via std library logger using ProcessorFormatter.

@@ -495,9 +495,9 @@ def test_foreign_delegate(self, configure_for_pf, capsys):

assert ("", "foo [in test_foreign_delegate]\n") == capsys.readouterr()

def test_clears_args(self, capsys, configure_for_pf):
def test_clears_args(self, configure_for_pf, capsys):

This comment has been minimized.

@ZlatyChlapec

ZlatyChlapec Nov 30, 2018
Author Contributor

I changed this so it's according to style in whole class but I'll remove the change if that's a problem.

tests/test_stdlib.py Outdated Show resolved Hide resolved
@ZlatyChlapec
Copy link
Contributor Author

@ZlatyChlapec ZlatyChlapec commented Nov 30, 2018

I am not sure what to do about failed CI. Looks like it just failed to execute.

@hynek
Copy link
Owner

@hynek hynek commented Dec 2, 2018

Hehe this is an almost 1:1 dupe of #188. :) Double spacing is on purpose btw. There's mixed opinions on it, I personally usually double space in fixed-width context like source code..

@ZlatyChlapec
Copy link
Contributor Author

@ZlatyChlapec ZlatyChlapec commented Dec 2, 2018

My bad I didn't check if there isn't anything about it before making PR. I changed it to try..except. Feel free to close in favor of #188.

@hynek
Copy link
Owner

@hynek hynek commented Jan 27, 2019

Looks like #188 is stalled for now. Would you like to finish this up?

@ZlatyChlapec
Copy link
Contributor Author

@ZlatyChlapec ZlatyChlapec commented Jan 27, 2019

Definitely, I would love that. What's next?

@hynek
Copy link
Owner

@hynek hynek commented Feb 2, 2019

Ah you know what? Nothing. :) I prefer the try/except way anyways and I'll fix the minor stuff myself. Thank you for your contribution and sorry it took so long!

@hynek hynek merged commit 8917d5d into hynek:master Feb 2, 2019
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
hynek added a commit that referenced this pull request Feb 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants