-
Notifications
You must be signed in to change notification settings - Fork 668
Fix all issues reported by running: tox -e pep8 and enable pep8 as a linter check #1397
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
Fix all issues reported by running: tox -e pep8 and enable pep8 as a linter check #1397
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1397 +/- ##
==========================================
+ Coverage 80.10% 80.15% +0.04%
==========================================
Files 73 73
Lines 4061 4066 +5
==========================================
+ Hits 3253 3259 +6
+ Misses 808 807 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Awesome! Just a few nitpicks from me, and I think that you might get a few more violations if you resolve conflicts from master as I just merged a few things 😊
Also this closes #1368 IMO, at least assuming the default tox run, I've updated the PR for that. I agree about |
Fixes to resolve errors for: https://www.flake8rules.com/rules/E741.html Do not use variables named 'I', 'O', or 'l' (E741) https://www.flake8rules.com/rules/E742.html Do not define classes named 'I', 'O', or 'l' (E742)
E712: Comparison to true should be 'if cond is true:' or 'if cond:' https://www.flake8rules.com/rules/E712.html
Thanks for the feedback. I just did a quick rebase. Will work on updating based on your comments. |
E711: Comparison to none should be 'if cond is none:' https://www.flake8rules.com/rules/E711.html
F401: Module imported but unused https://www.flake8rules.com/rules/F401.html
Local variable name is assigned to but never used https://www.flake8rules.com/rules/F841.html
I think I have updated everything you requested. |
@max-wittig could we disable the black check and enable This runs all the linters from tox which also avoids our previous issue with the black action having no proper versioning atm :) |
These are really nice little fixes 👍 @nejch I think, I can just force merge and next time, the Github actions thingy will adjust to the new config, right? |
I think someone has to fix the "expected check" in the Gitlab settings. https://docs.github.com/en/github/administering-a-repository/managing-a-branch-protection-rule I just rebased: #1352 And it shows that "black" is an "expected check". |
Fixed all the issues reported by
flake8
when runningtox -e pep8
.Separated them into individual commits for fixing different reported
flake8
errors.Finally enabled the
tox -e pep8
to run as part of the CI linting process.NOTE
In the F841 patch, I deleted the line:
parser = _get_base_parser(add_help=False)
As it doesn't seem have any impact. Please correct me if I am wrong.
Closes #1368.