-
Notifications
You must be signed in to change notification settings - Fork 39
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
Refactor logging configuration #933
Conversation
nice one @stefsmeets - I think this is a cleaner more portable approach! One remark - would you be able to write a couple tests for the new module, it's good to have it test-covered as a module even if (I think) there were minimal tests before 👍 🍺 |
Yeah, I could not find any tests so I did not bother but I can add a couple next week. 👍 |
cheers dude! Lemme know if you got time, if not I can do that, been doing a lot of maintenance and test stuff lately anyway 👍 |
Hi @valeriupredoi , I added a couple of tests to cover the functionality of module. Does this work for you? |
very cool @stefsmeets - 100% coverage too 🍺 |
Description
This code moves the logging out of the
_config.py
module into its own_logging.py
, so that the logger can be imported without initializingDIAGNOSTICS_PATH
/CFG
/ etc. I also did some refactoring and added some documentation based on my understanding of this module. No functionality has been added or changed.I hope that this will make it easer to work with. This is also needed for #907 for I'm looking to see if I can merge logging from there with the logging config that already exists.
_config.py
#889Before you get started
Checklist
- [ ] Documentation is available for new functionality- [ ] YAML files passpre-commit
oryamllint
checks- [ ] Unit tests are availableTo help with the number pull requests: