Skip to content

Conversation

@hjmjohnson
Copy link
Member

Move the pre-commit hooks from bash to python.

A few advantages of python:

  • Easier to maintain and understand
  • Easier to add complex logic and auto-fixes

PR Checklist

@hjmjohnson hjmjohnson marked this pull request as draft June 21, 2025 23:52
@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class labels Jun 21, 2025
@hjmjohnson hjmjohnson force-pushed the python-commit-message-update branch from 56505f0 to 756edb1 Compare June 21, 2025 23:53
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Mostly looks good on a glance.

@dzenanz dzenanz mentioned this pull request Jun 23, 2025
2 tasks
@thewtex thewtex requested a review from Copilot June 23, 2025 18:02

This comment was marked as outdated.

@hjmjohnson hjmjohnson marked this pull request as ready for review June 23, 2025 22:11
@hjmjohnson hjmjohnson changed the title WIP: REQUEST COMMENTS Python commit message update COMP: Python commit message update Jun 23, 2025
@hjmjohnson hjmjohnson self-assigned this Jun 23, 2025
@hjmjohnson hjmjohnson force-pushed the python-commit-message-update branch from 756edb1 to e4f0eee Compare June 23, 2025 22:13
@hjmjohnson hjmjohnson requested a review from Copilot June 23, 2025 22:13
@github-actions github-actions bot added the type:Compiler Compiler support or related warnings label Jun 23, 2025

This comment was marked as outdated.

@hjmjohnson hjmjohnson force-pushed the python-commit-message-update branch from e4f0eee to fdda390 Compare June 23, 2025 22:17
@hjmjohnson hjmjohnson requested a review from Copilot June 23, 2025 22:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the commit message hook from bash to Python to improve readability and maintainability.

  • Introduces a new Python script for the commit message hook in Utilities/Hooks/kw-commit-msg.py.
  • Removes the old bash-based hook in Utilities/Hooks/kw-commit-msg.
  • Updates the pre-commit configuration to reference the new Python script.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Utilities/Hooks/kw-commit-msg.py New Python implementation for commit message validation
Utilities/Hooks/kw-commit-msg Removal of the legacy bash hook
.pre-commit-config.yaml Updated hook entry to point to the Python script

The logic for testing and implementing commit
message testing is much more streamlined and
easier to maintain in the python implementation.

With the use of pre-commit hooks, the availaility
of python is already implied.
@hjmjohnson hjmjohnson force-pushed the python-commit-message-update branch from fdda390 to 46290e0 Compare June 23, 2025 22:20
@blowekamp
Copy link
Member

Should we move these hooks to a separate repo to be reused by projects?

@hjmjohnson
Copy link
Member Author

Should we move these hooks to a separate repo to be reused by projects?

I think they are easy enough to copy and "vimdiff" compare that the overhead of a separate project is not worth it. .... that's just my 2 cent opinion :).

@hjmjohnson hjmjohnson merged commit 45b4e6c into main Jun 24, 2025
16 checks passed
@hjmjohnson hjmjohnson deleted the python-commit-message-update branch June 24, 2025 13:38
@jhlegarreta
Copy link
Member

@jhlegarreta
Copy link
Member

jhlegarreta commented Jul 18, 2025

Re #5434 (comment) seems like the SO question was not very popular, but digging more into this I found this:
https://jorisroovers.com/gitlint/latest/

which is supported by pre-commit. It seems to have many relevant features, e.g.
https://github.com/jorisroovers/gitlint/blob/4d9119760056492eabc201bfad5de2f9e660b85f/examples/gitlint#L26
and
https://github.com/jorisroovers/gitlint/blob/4d9119760056492eabc201bfad5de2f9e660b85f/examples/gitlint#L35C1-L35C20

I may have a better look if we agree that we want to adopt this.

@thewtex
Copy link
Member

thewtex commented Jul 22, 2025

gitlint sounds promising

@jhlegarreta
Copy link
Member

Had a closer look and it seems like it is no longer maintained, unfortunately (last commit was 2 years ago).

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

Labels

area:Python wrapping Python bindings for a class type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants