-
Notifications
You must be signed in to change notification settings - Fork 123
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
Fix/logging #2118
Fix/logging #2118
Conversation
* remove self assignation when using addfile_handler and add_stdout_handler
… with the default file name doesn't work * Fix check for hasHandlers
Codecov Report
@@ Coverage Diff @@
## main #2118 +/- ##
==========================================
- Coverage 86.43% 86.43% -0.01%
==========================================
Files 45 45
Lines 7971 7985 +14
==========================================
+ Hits 6890 6902 +12
- Misses 1081 1083 +2 |
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.
Some questions regarding:
Add types to logging.py
Good job!
remove self assignation when using addfile_handler and add_stdout_handler
I understand those assignations were redundant.
Fix bug: Trying to instantiate a logger and redirect to a file with the default file name doesn't work
Do you mean this line to_file or filename != FILE_NAME:
??
Fix check for hasHandlers
Thank you!
Fix call on _make_child_logger
This is the same as Fix check for hasHandlers
, am I right?
Exactly, yes
Not really, L488 you called |
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.
LGTM. Thank you Jamil!
Add types to logging.py
remove self assignation when using
addfile_handler
andadd_stdout_handler
Fix bug: Trying to instanciate a logger and redirect to a file with the default file name doesn't work
Fix check for
hasHandlers
Fix call on
_make_child_logger