-
-
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
Support duplicate-code
message with --jobs
#8757
Support duplicate-code
message with --jobs
#8757
Conversation
duplicate-code
message with --jobs
duplicate-code
message with --jobs
@@ -233,7 +233,6 @@ def test_parallel_execution(self) -> None: | |||
join(HERE, "functional", "a", "arguments.py"), | |||
], | |||
out=out, | |||
# We expect similarities to fail and an error |
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.
No reason to expect similarity to fail when checking only one file. Maybe an earlier version of this test checked the same file twice?
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.
The comment was very unclear, sorry about that.
It was meant to say something like:
if we test the same file twice, which this test was doing, in parallel mode we get extra errors, so we only test a single file here in parallel mode, ensuring it errors. All parallel checks are in their own file now.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8757 +/- ##
=======================================
Coverage 95.82% 95.82%
=======================================
Files 173 173
Lines 18386 18386
=======================================
+ Hits 17618 17619 +1
+ Misses 768 767 -1
|
@@ -130,7 +130,7 @@ def check_parallel( | |||
"""Use the given linter to lint the files with given amount of workers (jobs). | |||
|
|||
This splits the work filestream-by-filestream. If you need to do work across | |||
multiple files, as in the similarity-checker, then implement the map/reduce mixin functionality. |
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.
is it no longer [considered] a mix-in class?
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.
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.
Oooooh, hadn't seen those, sexy
""" | ||
Similar.combine_mapreduce_data(self, linesets_collection=data) | ||
self.close() |
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.
Simple as that is it! Great stuff!! :)
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.
Holy mother of Guido, was it that simple ?!
I just want to see the primer result with |
dc5f010
to
0a88e97
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.
Haven't looked at the code but great find Jacob!
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.
This is such an elegant fix to a problem that have been plaguing pylint for a decade, amazing ! Thanks also to @doublethefish who did almost all the work minus that one self.close()
in 2021π
IIRC I tried adding |
Type of Changes
Description
All the groundwork for detecting
duplicate-code
with--jobs
was laid in #4007, but it was only unit tested without actually checking that messages were emitted.Closes #374 (I moved the remaining cyclic-import issue to #4171)