Many teams use pre-commit to enforce code style before pushing up changes. This project seems to have some checks and bots after push. Before push is a bit better, I think. Quicker dev loop.
It would require every developer to install pre-commit and run "pre-commit install" on their cloned repo, tho. So, we'd all have to agree.
Current State
No pre-commit
Expected State
pre-commit runs on every git commit
Implementation Plan
Add proposed .pre-commit-config.yaml
Possibly add pre-commit hooks for typescript
run "pre-commit run --all" on all the code
Optionally exclude the git hash from blame in .git-blame-ignore-revs if we are fussy about git blame records
Add pre-commit command to github workflow to check that pre-commit passes, so no one can accidentally forget to install pre-commit and push up something broken. I'm not sure where to add it.
Relevant Code Snippets
# A sample pre-commit
# See http://pre-commit.com/#python
# See https://github.com/pre-commit/pre-commit-hooks
# Run 'pre-commit install' to install the pre-commit hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.6.0
hooks:
- id: check-added-large-files
- id: check-ast
- id: check-case-conflict
- id: check-docstring-first
- id: check-merge-conflict
- id: check-symlinks
- id: debug-statements
- id: detect-private-key
- id: end-of-file-fixer
- id: mixed-line-ending
- id: trailing-whitespace
exclude: (.csv|.tsv)$
- id: pretty-format-json
args: ['--no-sort-keys', '--autofix']
# don't commit directly to main or master
- id: no-commit-to-branch
- repo: https://github.com/pre-commit/mirrors-prettier
rev: "v4.0.0-alpha.8"
hooks:
- id: prettier
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.6.8
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
- id: ruff-format
## ruff does not re-implement all of pylint, see https://github.com/astral-sh/ruff/issues/970
- repo: https://github.com/PyCQA/pylint
rev: v3.3.1
hooks:
- id: pylint
language_version: python3.11
args:
# ruff is controlling line length:
- --disable=line-too-long
# let's not worry about these for now
- --disable=duplicate-code
- --disable=fixme
- --disable=import-error
- --disable=logging-fstring-interpolation
- --disable=missing-class-docstring
- --disable=missing-function-docstring
- --disable=missing-module-docstring
- --disable=no-member
- --disable=too-few-public-methods
- --disable=too-many-instance-attributes
- --disable=too-many-arguments
- --disable=too-many-branches
- --disable=too-many-locals
# black is taking care of import order:
- --disable=wrong-import-order
# TODO: re-enable these
- --disable=unused-argument
- --disable=protected-access
- --disable=invalid-name
- --disable=raise-missing-from
- --disable=too-many-positional-arguments
- repo: https://github.com/Lucas-C/pre-commit-hooks
rev: v1.5.5
hooks:
- id: forbid-crlf
# don't just remove, seems dangerous
# - id: remove-crlf
- id: forbid-tabs
exclude: (README.md|.tsv)$
# don't just remove, seems dangerous
# - id: remove-tabs
Why is this issue important?
This is just an idea for discussion.
Many teams use pre-commit to enforce code style before pushing up changes. This project seems to have some checks and bots after push. Before push is a bit better, I think. Quicker dev loop.
It would require every developer to install pre-commit and run "pre-commit install" on their cloned repo, tho. So, we'd all have to agree.
Current State
No pre-commit
Expected State
pre-commit runs on every git commit
Implementation Plan
Relevant Code Snippets