-
Notifications
You must be signed in to change notification settings - Fork 8
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
213 feature request switch from log class to python logging module #312
213 feature request switch from log class to python logging module #312
Conversation
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.
Introduction of logging - changes are clear and also have been tested. There are possibly other areas of the code where print statements can be replaced.
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.
Should we replace other print statements in this file?
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 believe the print statements here should remain as they are, if removed, key informations would not be printed to video for the users. I added a few missing calls to log.info though to have a more complete log
Codecov ReportAttention: Patch coverage is
|
Description
The custom Log class has been eliminated and the python standard logging module has been implemented instead.
logging.INFO
level messages are sent to the log file (same directory as before) whilelogging.WARNING
level messages only are sent to console. To be reviewed after merge of PR #300Fixes #213
Type of change
Please select what type of change this is.
Testing
CI testing has been added as well as manual testing has been performed.
Checklist: