[CI] pre-commit: run maven-spotless-apply and oxipng manually
#2197
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The "manual" stage in pre-commit refers to a specific hook stage designed for hooks that are not intended to run automatically during a standard git commit operation. Instead, these hooks are meant to be triggered explicitly by a user, typically when performing a full repository scan or a specific check. This speeds up the standard `pre-commit run --all-files` run by not running these two hooks. Both hooks are time consuming Can run the manual hooks with: `pre-commit run --all-files --hook-stage manual`
|
The pre-commit run here only took about 2 minutes. On other previous PRs it takes about 5 minutes |
jbampton
commented
Jul 30, 2025
There was a problem hiding this comment.
This removes the automatic quality gates for code formatting, image optimization during a standard commit. This can lead to inconsistencies and quality issues being introduced into the codebase if developers forget to run the manual hooks.
Recommendation:
To ensure these checks are still performed, please consider adding a CI step that runs the manual hooks on every pull request. This would look something like:
- name: Run manual pre-commit hooks
run: pre-commit run --all-files --hook-stage manual
This approach balances developer convenience with repository quality.
|
I still think maven spotless apply should run automatically, just that ideally we don't run it unnecessarily (e.g during python development). Why don't we just remove this part? always_run:
true # Ensures this hook runs even if no Java files are changed.
# This is useful for spotless:apply which might affect files not staged.If we remove it, won't pre-commit still run whenever changes to java/scala files are committed? That's really the only time it matters right IMO. Or am I missing something? I don't fully understand the intention of the comment. |
|
I tried your suggestion against the master branch and it didn't seem to work. I get: If it worked it would not fail and would print for the spotless check And I did try messy Java files and staged them and had the same result that hook did not apply. See below for my workflow. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The "manual" stage in pre-commit refers to a specific hook stage designed for hooks that are not intended to run automatically during a standard git commit operation. Instead, these hooks are meant to be triggered explicitly by a user, typically when performing a full repository scan or a specific check.
This speeds up the standard
pre-commit run --all-filesrun by not running these two hooks.Both hooks are time consuming
Can run the manual hooks with:
pre-commit run --all-files --hook-stage manualDid you read the Contributor Guide?
Is this PR related to a ticket?
[CI] my subjectWhat changes were proposed in this PR?
As described above.
How was this patch tested?
Ran pre-commit both normally and manually.
Did this PR include necessary documentation updates?