-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[CI Regression?] Re-enable flake8 #2501
Comments
By the way, actually invoking |
Excluding a few dirs makes flake8 finish in 16s (that's over 5x faster). Invoking flake8 revealed some other configuration problems. It makes me think that flake8 wasn't run in CI long before #2486. |
This makes sure that flake8's defaults are in use. Specifically, it excludes `.tox/` dir that is known to contain a lot of files in nested folders that are not supposed to be linted. Refs: * pypa#2501 (comment) * https://github.com/pypa/setuptools/pull/2486/files#r546877674 * https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-extend-exclude * https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-exclude
It's working as intended for me:
Note, you don't need to pass Note that I also saw the skips on the second run. That's working as intended. The flake8 tests don't need to be run a second time if the files haven't changed since they were last checked. Also, you can see the tests ran in 13s for me, and 1.8s for the second attempt. I did disable coverage for these runs to avoid the noisy (buggy) output that coverage generates. |
It's interesting that I didn't see any PASSED lines and that's what I'm confused about. Still, I've found a number of misconfigured settings in the configs. Also, I think that |
I've made an intentional syntax error in |
If you run Agreed, it would be nice for flake8 to run naturally, though I'm okay with that usage being broken as long as the test suite is able to enforce the flake8 checks during the test run. I believe the use of extend-ignore (jaraco/skeleton#28) should address that concern, as you describe. |
I've verified that making style violations adds the failed entries and fixing that produces PASSED. I didn't realize there's some cache. Anyway, it's quite confusing to see linters among regular tests.
Almost. Also, there's |
FWIW I'll probably send a few PRs later to address what I've noticed. |
Sounds good. For this first round, I'm aiming toward simplicity and conformance with the skeleton without unduly compromising the project's expectations. I'd like to let these changes stabilize before tweaking them further. Feel free to send PRs, but I may choose to sit on them for some time, especially if they're not critical changes. |
skeleton PR: jaraco/skeleton#33 |
This makes sure that flake8's defaults are in use. Specifically, it excludes `.tox/` dir that is known to contain a lot of files in nested folders that are not supposed to be linted. Refs: * pypa#2501 (comment) * https://github.com/pypa/setuptools/pull/2486/files#r546877674 * https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-extend-exclude * https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-exclude
setuptools PR: #2508 |
Invalid config entries PR: #2509. |
This option allows adding extra ignored rules to the default list instead of replacing it. The default exclusions are: E121, E123, E126, E226, E24, E704, W503 and W504. Refs: * https://github.com/pypa/setuptools/pull/2486/files#r541943356 * https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-extend-ignore * https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-ignore * pypa#2501 * https://github.com//skeleton/issues/28
It looks like this issue has been addressed. |
@jaraco it looks like after #2486 flake8 is always skipped:
The text was updated successfully, but these errors were encountered: