You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add yourself to CONTRIBUTORS if you are a new contributor.
Add a ChangeLog entry describing what your PR does.
If it's a new feature, or an important bug fix, add a What's New entry in doc/whatsnew/<current release.rst>.
Write a good description on what the PR does.
Type of Changes
Type
✓
✨ New feature
Description
Please read
The most important reason why nobody could get this to work (I think) is because the similarity check can be ran stand-alone. I'm not sure what the rationale for this is, but I think that might have caused issues for other contributors.
This PR checks whether self has linter attribute (and therefore is not being run standalone) and then checks for disables. I think this is a fair approach to the problem: if you're running the similarity checker on its own it shouldn't be disabled by pylint type comments.
I would like the opinion of maintainers though as looking at the issues and past PRs has shown that this discussion has been quite opinionated.
This PR checks whether self has linter attribute (and therefore is not being run standalone) and then checks for disables. I think this is a fair approach to the problem: if you're running the similarity checker on its own it shouldn't be disabled by pylint type comments.
Is there no way to easily take the disable into account when launched as a standalone ? If not I think it's fair enough, being able to disable when run with pylint is one of the most asked feature so having this one is pretty important.
This PR checks whether self has linter attribute (and therefore is not being run standalone) and then checks for disables. I think this is a fair approach to the problem: if you're running the similarity checker on its own it shouldn't be disabled by pylint type comments.
Is there no way to easily take the disable into account when launched as a standalone ? If not I think it's fair enough, being able to disable when run with pylint is one of the most asked feature so having this one is pretty important.
Well, the issue with that is that the disable param checking is all done on the PyLinter class. In standalone mode no PyLinter class is instantiated so we would need to write a completely separate piece of code to handle this. I think this is why the previous attempts at this all failed: you need to duplicate a lot of work from PyLinter for this to be able to run in standalone and in pylint mode with the same form of disabling.
Maybe we can instead instantiate a PyLinter with --disable=all --enable=duplicate-code ?
Haven't looked into that, but that kind of defeats the purpose of having this be a stand-alone tool. You would then also get all the overhead of configuration loading etc.
Personally I think we should be fine with adding this and then even closing the issue.
If somebody really wants to disable the duplicate-code checker while they are running symilar stand-alone (why even run it in the first place then...) they can create a new issue and we can then discuss if there is a real use case for that.
Right, it's possible to launch pylint with --disable=all --enable=duplicate-codedirectly instead. And it's also possible to use a specialized tool like Simian or PMD directly
There's currently 121 👍 on the original issue, safe to say (1) you're going made a lot of person happy with this one 😄
(1) unless a catastrophic failure condition eluded me during review 😂
Yeah I completely forgot about this, but my response saying that I had a potential fix also got 12 👍... I'll do one final review as this was quite long ago and then I'll merge (if that's okay with you of course!).
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DanielNoord
added
the
Needs review 🔍
doc/whatsnew/<current release.rst>.Type of Changes
Description
Please read
The most important reason why nobody could get this to work (I think) is because the similarity check can be ran stand-alone. I'm not sure what the rationale for this is, but I think that might have caused issues for other contributors.
This PR checks whether
selfhaslinterattribute (and therefore is not being run standalone) and then checks for disables. I think this is a fair approach to the problem: if you're running the similarity checker on its own it shouldn't be disabled bypylinttype comments.I would like the opinion of maintainers though as looking at the issues and past PRs has shown that this discussion has been quite opinionated.
Potentially closes #214