Simplify Makefile and requirements #5145
Conversation
| - id: requirements-txt-fixer | ||
| - id: file-contents-sorter | ||
| files: CONTRIBUTORS.txt | ||
| # Another entry is required to apply file-contents-sorter to another file | ||
| - repo: https://github.com/pre-commit/pre-commit-hooks | ||
| rev: 'v3.3.0' | ||
| hooks: | ||
| - id: file-contents-sorter | ||
| files: docs/spelling_wordlist.txt | ||
| - repo: https://github.com/asottile/pyupgrade | ||
| rev: 'v2.7.3' | ||
| hooks: | ||
| - id: pyupgrade | ||
| args: ['--py36-plus'] |
There was a problem hiding this comment.
Putting the formatters at the end is usually a bad idea. Because the linters will then check the contents as they were before formatting making it rather inconsistent.
There was a problem hiding this comment.
I've pushed flake8 at the last position, all previous hooks eighter modifies python source or don't check them
There was a problem hiding this comment.
Oh, and another piece of advice: you can split the hooks from the same repo into two and put in different places if needed. Within the blocks of mutating and non-mutating checks, I normally sort the individual hooks so that the fastest ones would go first. This maximizes the responsiveness of the process.
There was a problem hiding this comment.
One more thing: you can have a separate pre-commit config for very heavy checks and would be nice to always run in CI but users would only invoke them on-demand.
Makefile
Outdated
| lint: isort-check black-check flake8 mypy | ||
| .PHONY: pre-commit | ||
| pre-commit: | ||
| pre-commit run --all-files |
There was a problem hiding this comment.
Here's a recommended invocation example: https://github.com/ansible/pylibssh/blob/bd53dd8/tox.ini#L236. It's especially useful to have --show-diff-on-failure in CI, less so locally but it also comes in handy to review what mutations have been applied.
Also, dunno how it's possible with make but in tox it's common to allow end-users to bypass their own args to pre-commit so while tox -e lint runs pre-commit run --all-files, tox -e lint -- flake8 --files x/y/z would run pre-commit run flake8 --files x/y/z which is useful when trying to go through what a specfic linter reports separately in a more responsive manner.
Codecov Report
@@ Coverage Diff @@
## master #5145 +/- ##
=======================================
Coverage 97.56% 97.56%
=======================================
Files 43 43
Lines 8780 8780
Branches 1412 1412
=======================================
Hits 8566 8566
Misses 100 100
Partials 114 114
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
💔 Backport was not successfulThe PR was attempted backported to the following branches:
|
No description provided.