Add option to allow linting over all files #4217
Conversation
A new flag was added to enable linting over all files in and under a directory. By default, it will not alter pylint current behavior to process only traditional and namespace packages. Signed-off-by: Tiago Honorato <tiagohonorato1@gmail.com>
|
I think this is addressing the highest voted and oldest still opened issue in pylint : #352, so this is much appreciated. We'll want this to be the default behavior and not an option because it feels natural for pylint to work like this. But we're going to need a lot more automated tests to make sure it's working, see in particular @AWhetter comment here: #352 (comment) |
|
Hi @Pierre-Sassoulas, I think we all would appreciate this feature, so happy to help here! I totally agree with you on that linting all files definitely feels more natural. I decided to implement as an option just as a starting point since I was following the general idea and guidelines from #352, specially this comment from @PCManticore. I recall reading the comment you mentioned, I will take a closer look and we can go from there. |
|
Ho yeah, let's keep the flag to deactivate it if needed, that's safer actually. |
|
@Pierre-Sassoulas, after looking a bit further, as we don't have any tests checking |
Pierre-Sassoulas
added
Enhancement ✨
|
I added some tests for expand_modules in #4283, I think this is just the beginning but everything seems to happen here and I have a feeling things can be simplified. Please let me know what you think :) |
Hi @Pierre-Sassoulas, thank you for helping here! I will rebase and take a closer look, but from what I could already notice, I really liked your approach to the unittest. I was having a lot of trouble to come up with something clean! |
|
@Pierre-Sassoulas Is there anything I can do to help this forward? I think this might be one of the most requested features within |
|
Hey @DanielNoord you're right this is one of the most wanted feature, for good reasons. I think it's safe to say this MR is not moving right now, so if you want to build on it or even to do you own fix from scratch that would be very much appreciated (You can start by checking |
|
The general idea of this change seems fine, but I'm worried about this comment: |
|
I don't think it was considered here and this is making everyone afraid of fixing this issue. But do we have to consider it to fix the issue ? Right now if you want to lint all files in a directory you can gives the file one by one (pre-commit does that) . Maybe there are unconsistencies if you do that instead of giving a directory containing the file but this is another issue imo. |
|
I was looking at #2967 as well, as they all seem related: |
|
Closing in favor of #5682 |
Steps
doc/whatsnew/<current release.rst>.Description
A new flag
--lint-all=<yn>was added to enable linting over all files in and under a directory. By default, it will not alter pylint current behavior to process only traditional and namespace packages.I have implemented some tests, but I feel that maybe it is worth going a bit further by, for example, checking how many files were expected to be checked and comparing with the number of files that were actually checked.
Signed-off-by: Tiago Honorato tiagohonorato1@gmail.com
Type of Changes
Related Issue
#352