-
-
Notifications
You must be signed in to change notification settings - Fork 683
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
Add CustomLogger
to prevent IDE lint warnings when calling log.trace
.
#1831
Conversation
Here is the |
There's also the All other changes are removing the old monkeypatch import logging
log = logging.getLogger(__name__) to from bot.utils.logging import get_logger
log = get_logger(__name__) |
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.
Nice, I've personally wanted this for a while 👍🏻
Missing log migration: bot/exts/recruitment/talentpool/_cog.py
NB: Linting & Test checks currently failing due to maintenance on the coveralls API. Will rerun checks later 👍 Edit: coveralls has now been removed (see #1838) so just waiting for reviews now |
NB: The |
requested changes have been pushed.
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'm not sure using log = logging.getLogger(__name__)
is the best solution to the circular import problem, although i'm not sure what is either...
Other than that this looks good.
b7cc69f
to
4b8aea9
Compare
Have fixed the root issue of the circular imports, so now all files should be using Also added any missing migrations from PRs made after I started this and squashed, fixed the conflicts (all 25), split the initial commit into two (one for creating the new class and second for the migrations), and did a commit for making newlines after imports consistent (part of PyCharm's "Optimize Imports" feature). This should now be fully read for review @kosayoda @wookie184 👍 |
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.
Two small things, other than that it looks good.
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.
All seems good to me 👍
89d3e8a
to
2e6239a
Compare
There's now always one blank line (no more, no less) after an import
2e6239a
to
c9bfe51
Compare
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.
Approving on @ChrisLovering's behalf.
And also my own.
Closes #1830
Implements the proposed
CustomLogger
class to fix the lint warnings.