-
Notifications
You must be signed in to change notification settings - Fork 61
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 unnecessary sting concatenation in logging statements; downgrade logging level to INFO in BAU places #370
Conversation
wow, thanks @ylexus, exactly the kind of review the logging needed! looks good af first sight, will dive in a bit deeper one of the next days |
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.
Thanks for the cleanup, but you changes some of the loggers from warning to info. Please undo that.
Yes, those changes to INFO was the whole point of this, the rest is just a side effect (automatic refactoring). Apologies, I should have mentioned the original issue is in the PR title and not in the first comment. |
You mixed two things in the same commit. Can you please change that, then i'll merge your request. Commits should clearly define what they change, which is not the case here |
@eitch done |
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.
looks good
Thanks, how often do you do releases? Just to know when to expect this change to be released. |
As often as needed. We just did a 2.6.1. We might do a 2.6.2 soon. |
#369