Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upcibuild: Add flake8 tests for Python syntax errors #441
Conversation
| mock | ||
| nose | ||
| pycodestyle==2.4.0 | ||
| pyflakes==1.6.0 |
This comment has been minimized.
This comment has been minimized.
ross
Jan 6, 2020
Contributor
Would prefer to leave these in here to pin versions (feel free to update the version to the current/latest.)
We've run in to problems in the past where CI starts failing unrelated to changes in PRs due to updates the CI process grabbing a newer version than the codebase has been vetted by. (new checks added or existing updated.)
This comment has been minimized.
This comment has been minimized.
cclauss
Jan 6, 2020
•
Author
Contributor
I tend to pin runtime dependencies for repeatable builds but allow testing tools, linters, formatters, etc. to float. As new tests are added/improved, the testing gets more rigorous and more issues that should get fixed are fixed which improves code quality and resiliency. It does not matter if the issue is related to a particular PR. What matters is the code continues to be improved.
Take pyflakes for instance... Here it is hardcoded so none of the improved tests between 1.6 and 2.1.1 have been run on this codebase. That is unfortunate because those tests could have improved code quality and resiliency.
This comment has been minimized.
This comment has been minimized.
ross
Jan 6, 2020
Contributor
Understand that, but the decision was made to trade that off and to update the pinned versions from time to time.
This comment has been minimized.
This comment has been minimized.
ross
Jan 6, 2020
Contributor
Also note that if you go back in time and check out a specific release tag or SHA and install requirements it'll fail to pass linting b/c that code/tag wasn't tested with versions of things that didn't exist at the time. Having them pinned keeps released tested and validated with the version in use at the time.
I expect dependabot to be able to file PRs to update these versions as the come available in the hopefully not too distant future which should get things to the ideal set up.
| @@ -17,5 +17,6 @@ fi | |||
|
|
|||
| SOURCES="*.py octodns/*.py octodns/*/*.py tests/*.py" | |||
|
|
|||
| pycodestyle --ignore=E221,E241,E251,E722,W504 $SOURCES | |||
| pycodestyle --ignore=E117,E221,E241,E251,E722,W504 $SOURCES | |||
This comment has been minimized.
This comment has been minimized.
ross
Jan 6, 2020
Contributor
What's the reason for adding E117. That check get updated to be more strict/different?
This comment has been minimized.
This comment has been minimized.
cclauss
Jan 6, 2020
•
Author
Contributor
E117 over-indented seems to disagree with the code formatter that this project uses.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
As recommended by @ross at github#441 (comment)
As recommended by @ross at github#441 (comment)
| @@ -19,3 +19,4 @@ SOURCES="*.py octodns/*.py octodns/*/*.py tests/*.py" | |||
|
|
|||
| pycodestyle --ignore=E221,E241,E251,E722,W504 $SOURCES | |||
| pyflakes $SOURCES | |||
| flake8 . --count --exclude=./env/* --select=E9,F63,F7,F82 --show-source --statistics | |||
This comment has been minimized.
This comment has been minimized.
ross
Jan 16, 2020
Contributor
Running into problems with the way this does . when I have extra virtualenvs in my working directory. It tries to run on all the code in them.
| flake8 . --count --exclude=./env/* --select=E9,F63,F7,F82 --show-source --statistics | |
| flake8 $SOURCES --count --select=E9,F63,F7,F82 --show-source --statistics |
Ideally Would like to remove the --select eventually and get all the warnings fixed. Can do that in subsequent PR(s).
cclauss commentedDec 31, 2019
•
edited
Also add flake8 tests for undefined names, etc. flake8 is a superset of both pycodestyle and pyflakes so installing flake8 in requirements-dev.txt will continue to install both pycodestyle and pyflakes.
https://flake8.pycqa.org/en/latest/user/error-codes.html
On the flake8 test selection, this PR does not focus on "style violations" (the majority of flake8 error codes that psf/black can autocorrect). Instead these tests focus on runtime safety and correctness: