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

Don't cache logger #72

Merged
merged 4 commits into from May 23, 2016
Merged

Don't cache logger #72

merged 4 commits into from May 23, 2016

Conversation

@glasserc
Copy link
Contributor

@glasserc glasserc commented May 23, 2016

Fix #71 by not caching any part of the configuration in the proxy unless explicitly requested. This makes the _logger attribute consistent with other parameters of the BoundLoggerLazyProxy such as _processors, _wrapper_class, and _context_class.

@hynek
Copy link
Owner

@hynek hynek commented May 23, 2016

Hm, I’d love to know why Travis is not picking up this change. I think GitHub reported some problems today.

Could you please add a docstring to the test in the style of https://jml.io/pages/test-docstrings.html ? I know that old tests don’t have them but I require them for new ones. Makes it much easier to understand.

Thanks!

@codecov-io
Copy link

@codecov-io codecov-io commented May 23, 2016

Current coverage is 100%

Merging #72 into master will not change coverage

@@             master        #72   diff @@
==========================================
  Files            13         13          
  Lines           694        695     +1   
  Methods           0          0          
  Messages          0          0          
  Branches         88         88          
==========================================
+ Hits            694        695     +1   
  Misses            0          0          
  Partials          0          0          

Powered by Codecov. Last updated by 9ae0c00...2f7eb7d

We don't use this variable.
@glasserc
Copy link
Contributor Author

@glasserc glasserc commented May 23, 2016

I've pushed a docstring and some flake8 fixes. Please let me know of any other improvements you'd like to see.

@hynek hynek merged commit b6236d4 into hynek:master May 23, 2016
2 checks passed
2 checks passed
codecov/project 100% compared to 9ae0c00
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hynek
Copy link
Owner

@hynek hynek commented May 23, 2016

thanks!

@Natim
Copy link

@Natim Natim commented May 24, 2016

@hynek Do you think it would be possible to build a patch release with this on pypi?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants