Converted python code to pylint compliant. #32
Merged
Conversation
This file contains 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
Shivani-chourasiya
force-pushed
the
pylint_pep8
branch
2 times, most recently
from
36a51d4 to
98a485f
Compare
gmuloc
requested changes
Jun 2, 2022
There was a problem hiding this comment.
Very good intiative!
This PR is mixing refactoring code functionality and linting so it is a bit hard to follow.
I cannot guarantee 100% that you are not changing some functionality. Were you able to test on a couple of examples at least?
A couple of points:
- don't use
\to break lines except in very special occasion (cf PEP8) - I am wondering if simply you dont want
@staticmethodrather than@classmethodeverywhere. I guess you were trying to escape the infamousmethod could be a functionpylint warning. Though in this case it makes life a bit less understandable. There is not really a reason for the checktext method to be a classmethod if the only place where we are using the class is in some debug log. - Sometimes pylint is having strong opinions that can decrease readability
gmuloc
requested changes
Jun 3, 2022
There was a problem hiding this comment.
What is the purpose of the log in the class? It looks like it was added only to avoid to trigger unused import - more logging is good but some of these log messages do not make sense.
What do you mean with Check * rule does not exist - are you saying that the rule was not triggered for this given line? If so it maybe make sense for the method that returns True / False but for the one returning a result or a regex it is not possible to assert before the return statement if the check exist or not and so the logs seem incorrect.
Two things:
- It is better to log when a rule is triggered in debug rather than when it is not I would argue
- If the only purpose of the log message is to cater for
unused-importI would not add it
Shivani-chourasiya
force-pushed
the
pylint_pep8
branch
3 times, most recently
from
75a4822 to
7da4e99
Compare
Shivani-chourasiya
force-pushed
the
pylint_pep8
branch
3 times, most recently
from
3f24bfb to
cdd2979
Compare
Shivani-chourasiya
force-pushed
the
pylint_pep8
branch
4 times, most recently
from
1d62470 to
a11bc34
Compare
gmuloc
requested changes
Jun 7, 2022
gmuloc
reviewed
Jun 8, 2022
gmuloc
reviewed
Jun 8, 2022
gmuloc
added a commit
that referenced
this pull request
Jun 8, 2022
Note: this is introducing pylint check in the workflow - the workflow is currently failing until #32 is merged into devel
gmuloc
requested changes
Jun 8, 2022
gmuloc
requested changes
Jun 10, 2022
Shivani-chourasiya
force-pushed
the
pylint_pep8
branch
from
3af39b9 to
d0f5d59
Compare
gmuloc
reviewed
Jun 13, 2022
gmuloc
reviewed
Jun 13, 2022
Co-authored-by: Guillaume Mulocher <[email protected]>
Co-authored-by: Guillaume Mulocher <[email protected]>
Co-authored-by: Guillaume Mulocher <[email protected]>
Co-authored-by: Guillaume Mulocher <[email protected]>
carlbuchmann
force-pushed
the
pylint_pep8
branch
from
4394628 to
0e6114b
Compare
* following pylint refactoring, either adjust the tests that are not valid anymore or remove them (2) * in one occasion fixed some refactoring issue (returning Rule instead of Rule() in the load_plugins function) * in one occasion set the correct error to be returned for a rule * fix ordering of steps for CLI run method for stdin
this is done to follow the new filename structure for the corresponding rules.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Updating the code to make it pylint compliant.