-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[base checker] Fix the ordering of checkers for extensions #8956
[base checker] Fix the ordering of checkers for extensions #8956
Conversation
The comparison was not done properly for extension when the second values given was a builtin and the first one an extension. The tests were not testing this case properly because the alphabetical order made us think that everything went right. Added a test with a low value extension checker. Necessary for pylint-dev#8951
2dc5b47
to
34c1139
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8956 +/- ##
=======================================
Coverage 95.74% 95.74%
=======================================
Files 173 173
Lines 18557 18558 +1
=======================================
+ Hits 17767 17768 +1
Misses 790 790
|
pylint/checkers/base_checker.py
Outdated
if not isinstance(other, BaseChecker): | ||
# print(f"{other} is not a base checker.") |
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.
Quite a bit of debug statement?
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 could turn that into comments, but then we'd need to transform it in debug statement again if there's a problem in the future π
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.
Hmm I think we could remove them here. This feels a bit weird to me
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.
Yeah true, comments explaining what the code is doing are bad form. (xor operator that makes it necessary must be bad form too π )
Type of Changes
Description
The comparison was not done properly for extension when the second values given was a builtin and the first one an extension. The tests were not testing this case properly because the alphabetical order made us think that everything went right. Added a test with a low alphabetical value extension checker.
Necessary for #8951