Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for pre-commit project pre-commit hooks #4235

Open
thewtex opened this issue Oct 1, 2023 · 8 comments
Open

Add support for pre-commit project pre-commit hooks #4235

thewtex opened this issue Oct 1, 2023 · 8 comments
Labels
type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots
Milestone

Comments

@thewtex
Copy link
Member

thewtex commented Oct 1, 2023

Description

Extend our pre-commit hook capabilities by adding support for the pre-commit project's pre-commit hooks.

Impact analysis

This will give use access to a number of community maintained pre-commit hooks. List of available hooks.

A few examples that we may want to enable:

  • clang-format
  • cppcheck
  • cpplint
  • include-what-you-use
  • cmake-format
  • cmake-lint
  • ruff
  • ruff-format

Expected behavior

The ./Utilities/SetupForDevelopment.sh script configures and enables the pre-commit project's pre-commit hooks.

Actual behavior

We currently only have our bash-based pre-commit hooks.

Versions

Target: ITK 6.

Environment

Should be cross-platform.

Additional Information

Note that this requires Python on the development system the pre-commit package.

Our current hooks setup could check for presence of Python or the pre-commit command. If so, we set up the pre-commit hook and support daisy-chaining the current hooks and the pre-commit hooks.

Then, incrementally enable pre-commit hooks we want to enable in its configuration.

@thewtex
Copy link
Member Author

thewtex commented Oct 4, 2023

To set up the python environment, we could possibly use micromamba.

@jhlegarreta
Copy link
Member

Applies to ITKSphinxExamples as well 👍, and maybe applicable to itk-wasm as well.

@thewtex
Copy link
Member Author

thewtex commented Oct 26, 2023

We may want to use cmake-micromamba to create a python environment for pre-commit. This could also be useful for remote modules to fetch dependencies.

@thewtex
Copy link
Member Author

thewtex commented Apr 12, 2024

Even lighter than micromamba is uv.

@blowekamp
Copy link
Member

blowekamp commented Apr 13, 2024

This has been done in SimpleITK:
https://github.com/SimpleITK/SimpleITK/blob/master/Utilities/GitSetup/setup-precommit
SimpleITK/SimpleITK#2069

The approach taken here was to use the pre-commit project as the primary master of the hooks, and call the KW hooks from it.

@thewtex
Copy link
Member Author

thewtex commented Apr 13, 2024

The approach taken here was to use the pre-commit project as the primary master of the hooks, and call the KW hooks from it.

Cool! We could use that + a pre-commit script to install uv if needed since uv is much faster and python3 is not available by default in Git Bash on Windows.

thewtex added a commit to thewtex/ITK that referenced this issue Jun 3, 2024
This never worked from PR pull requests due to GitHub permission
limitations. While it may be possible to fix this with recently added
GitHub Action permission features, a future replacement is planned based
on the `pre-commit` hooks, InsightSoftwareConsortium#4235 and related.
thewtex added a commit to thewtex/ITK that referenced this issue Jun 3, 2024
This never worked from fork PR pull requests due to GitHub permission
limitations. While it may be possible to fix this with recently added
GitHub Action permission features, a future replacement is planned based
on the `pre-commit` hooks, InsightSoftwareConsortium#4235 and related.
@thewtex
Copy link
Member Author

thewtex commented Jul 12, 2024

pixi is worth investigating to bootstrap access to a local python.

@thewtex
Copy link
Member Author

thewtex commented Dec 10, 2024

pixi is worth investigating to bootstrap access to a local python.

Added in: #5032

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots
Projects
None yet
Development

No branches or pull requests

3 participants