Make minor updates and tweaks to pre-commit setup#1384
Conversation
Setting `default_install_hook_types` makes it possible to tell people to simply run `pre-commit install` for installation, instead of having to supply the stage arguments.
These are based on experience with our other projects.
This makes it tailored for pytest tests.
This changes the message style so that they (hopefully) read more naturally.
Takes too long.
There was a problem hiding this comment.
Code Review
This pull request updates the pre-commit configuration (.pre-commit-config.yaml) and documentation (CONTRIBUTING.md) to simplify installation, refine hook names, and add new validation checks. Key changes include configuring default_install_hook_types so that a simple pre-commit install sets up all hooks, updating hook descriptions to be declarative, and adding new hooks like config-file-validator and validate-pyproject. Feedback on these changes highlights portability issues on Windows: specifically, using a local python hook with a bash script entry for shellcheck and overriding the check-jsonschema entry points with bash -c will both fail on Windows environments. The reviewer suggests using the official shellcheck-py hook and removing the unnecessary bash -c overrides to maintain cross-platform compatibility.
| - repo: local | ||
| hooks: | ||
| - id: shellcheck | ||
| name: 'Check shell scripts' | ||
| name: 'Shell scripts are lint-free' | ||
| entry: check/shellcheck | ||
| additional_dependencies: [shellcheck-py>=0.11.0] | ||
| language: python | ||
| files: '(\.sh$|^check/[^.]+$)' |
There was a problem hiding this comment.
Using language: python with a bash script (check/shellcheck) as the entry is not portable and will fail on Windows systems because Windows cannot execute bash scripts directly. To make this portable and avoid requiring users to manually install shellcheck, you can use the official shellcheck-py pre-commit hook repository, which automatically manages the Python-wrapped installation of shellcheck across platforms.
- repo: https://github.com/shellcheck-py/shellcheck-py
rev: v0.10.0.1
hooks:
- id: shellcheck
name: 'Shell scripts are lint-free'
files: '(\.sh$|^check/[^.]+$)'| - id: check-github-workflows | ||
| name: 'GitHub Actions workflow files are syntactically valid' | ||
| entry: | | ||
| bash -c 'exec check-jsonschema --builtin-schema vendor.github-workflows "$@"' -- | ||
| stages: [pre-push] | ||
|
|
||
| - id: check-github-actions | ||
| name: 'GitHub Actions definitions (if any) are syntactically valid' | ||
| entry: | | ||
| bash -c 'exec check-jsonschema --builtin-schema vendor.github-actions "$@"' -- | ||
| stages: [pre-push] |
There was a problem hiding this comment.
Overriding the entry with bash -c is unnecessary and breaks compatibility on Windows environments where bash is not available or not in the system PATH. The check-jsonschema hooks already define the correct entry points natively. You can achieve the same behavior portably by simply specifying the stages parameter without overriding entry.
- id: check-github-workflows
name: 'GitHub Actions workflow files are syntactically valid'
stages: [pre-push]
- id: check-github-actions
name: 'GitHub Actions definitions (if any) are syntactically valid'
stages: [pre-push]
This makes some minor updates and tweaks to
.pre-commit-config.yaml:Define the hook stages that should be installed, which simplifies the installation instructions to just
pre-commit installwithout having to list 3 separate stagesUse hadolint-py for the Docker hook instead of the previous hook; this one lets pre-commit automatically install a Python-wrapped installation of the hadolint program and saves the user from having to install hadolint manually.
Add
config-file-validatorhook.Remove the Markdown link checker hook because it took too long to run.
Add more common patterns to
exclude.Fix up some hooks
Update version hashes to latest versions as needed.
Change the style of the descriptions printed by pre-commit, to (hopefully) make them more natural.