Skip to content

Make minor updates and tweaks to pre-commit setup#1384

Open
mhucka wants to merge 10 commits into
quantumlib:mainfrom
mhucka:minor-pre-commit-tweaks
Open

Make minor updates and tweaks to pre-commit setup#1384
mhucka wants to merge 10 commits into
quantumlib:mainfrom
mhucka:minor-pre-commit-tweaks

Conversation

@mhucka

@mhucka mhucka commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

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 install without having to list 3 separate stages

  • Use 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-validator hook.

  • 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.

mhucka added 10 commits June 26, 2026 21:21
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.
@mhucka mhucka marked this pull request as ready for review June 26, 2026 23:11
@mhucka mhucka requested a review from arettig June 26, 2026 23:11

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .pre-commit-config.yaml
Comment on lines +130 to 137
- 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/[^.]+$)'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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/[^.]+$)'

Comment thread .pre-commit-config.yaml
Comment on lines +191 to 201
- 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]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant