Conversation
egpbos
left a comment
There was a problem hiding this comment.
Looks good to me: simple and clean!
One question: did you test this? I think installing PROTEUS in this repo's CI just to test logging would be overkill, but at least trying it out once on some machine would be good. It's not a blocker for merging imho, since fwl-io is still in active development anyway, and logging is already broken, so merging won't degrade anything in any case.
|
Yes! I did test it. Thank you for checking. I tested it by deleting some of the files in the FWL_DATA folder (the mass radius ones) which forced a re-download This causes fwl-io to create some logging calls, which then appear with the correct formatter/style in the terminal and PROTEUS log file. Perhaps I should have screenshotted this for the PR description above. I agree that, at some point soon, we should implement some robust testing into this repo which probes the interaction between fwl-io and other FWL modules. Thanks for the review! |
This pull request introduces a small update to correct the logging by updating logger names to include a package prefix. Closes #34