Based on discussions on Slack I suspect this may not be mergeable, but wanted to discuss anyway.
My priorities when implementing this were roughly prioritised as:
The hook should never block a commit that would pass all of the checks in isolation (no false positives—it shouldn't matter if the working directory is dirty, or because the hook implementation doesn't function correctly)
The hook should prevent commits that do not pass any of the checks that can run in a single-digit number of seconds. There should be no need for placate flake8 commits. (git commit --no-verify is right there if it is needed, but my strong opinion is that hooks need to introduce friction to be meaningful, otherwise they are like compiler and deprecation warnings and will be ignored until they do cause friction)
Per discussion in #240, avoid using .pre-commit-config.yaml
Per discussion on Slack, do not temporarily or otherwise destroy anything in the working directory.
Do not change anything in the commit (that should be a smudge/clean filter instead) or in the working directory (this is a difference from the .pre-commit-config.yaml way of doing things, because I usually get confused when I try to commit and it rewrites my stuff)
Things it checks:
Superfluous whitespace at EOL and EOF (by using git diff-index's built-in functionality)
Missing newlines at EOF (by doing some horrible parsing of git diff-index output)
black
flake8
mypy
Django check-migrations
building the documentation with make html
eslint
All of these apart from the first two require creating a copy of the repo in a temporary directory to be able to remove any files that the linters shouldn't check without affecting any open file handles in the working directory. (This also helps avoid polluting the reflog.) If creating that fails, these tests are skipped. The directory is removed before the hook finishes, including if it is aborted with Ctrl+C.
This requires a non-zero amount of machinery, but significantly less than used by .pre-commit-config.yaml, and is what I would consider the minimal required to satisfactorily run linters in a pre-commit hook (in a way I would be willing to use in my workflow).
This supersedes #240 and addresses #237
Based on discussions on Slack I suspect this may not be mergeable, but wanted to discuss anyway.
My priorities when implementing this were roughly prioritised as:
placate flake8commits. (git commit --no-verifyis right there if it is needed, but my strong opinion is that hooks need to introduce friction to be meaningful, otherwise they are like compiler and deprecation warnings and will be ignored until they do cause friction).pre-commit-config.yaml.pre-commit-config.yamlway of doing things, because I usually get confused when I try to commit and it rewrites my stuff)Things it checks:
git diff-index's built-in functionality)git diff-index output)blackflake8mypycheck-migrationsmake htmleslintAll of these apart from the first two require creating a copy of the repo in a temporary directory to be able to remove any files that the linters shouldn't check without affecting any open file handles in the working directory. (This also helps avoid polluting the reflog.) If creating that fails, these tests are skipped. The directory is removed before the hook finishes, including if it is aborted with Ctrl+C.
This requires a non-zero amount of machinery, but significantly less than used by
.pre-commit-config.yaml, and is what I would consider the minimal required to satisfactorily run linters in a pre-commit hook (in a way I would be willing to use in my workflow).