Skip to content

add git pre push hook #2553

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

lazer-dev
Copy link

@lazer-dev lazer-dev commented Jul 12, 2025

What this PR does

This PR adds support for installing a Git pre-push hook that runs spotlessCheck — and if formatting issues are found, it automatically runs spotlessApply to fix them, preventing broken pushes and helping developers keep code clean.

✅ Gradle plugin
New task: spotlessInstallGitPrePushHook

✅ Maven plugin
New goal: spotless:install-git-pre-push-hook

🛠 Shared core logic
Introduced GitPrePushHookInstaller class in the lib module

Why I added this

Formatting errors from spotlessCheck often show up only in CI/CD, which is frustrating.
Many developers (like myself) work from the command line:

git commit -m "message"
git push

They don’t always install IDE plugins or pre-commit tools.

This feature provides a simple way to install a Git pre-push hook that ensures formatting is always correct before pushing — without any manual setup or IDE dependency.

How the hook works

  1. The installed hook:
  2. Runs spotlessCheck
  • If issues are found:
  • It runs spotlessApply automatically
  • Prints a message: "Spotless found problems, running apply; commit the result and re-push"
  • Aborts the push with exit code 1

This way, developers get formatting fixes automatically, and can just commit the result and re-push.
No more broken builds due to formatting errors!

Summary of changes

✅ Added core utility: GitPrePushHookInstaller.java
✅ Gradle: spotlessInstallGitPrePushHook task
✅ Maven: spotless:install-git-pre-push-hook goal
✅ Updated CHANGES.md in:

  • plugin-gradle/CHANGES.md
  • plugin-maven/CHANGES.md
  • root CHANGES.md

Next steps

Let me know if:

  • you'd prefer the hook to be pre-commit instead
  • you'd like to make hook behavior (e.g. apply on fail) configurable
  • or want this feature disabled by default and opt-in via plugin config

Thanks for your time and feedback!

@nedtwigg
Copy link
Member

nedtwigg commented Jul 16, 2025

Thanks very much for this excellent contribution!

  • pre-commit vs pre-push
    • pre-commit hook has some problems (and a solution) outlined in spotlessApply --staged #623
    • pre-push avoid the worst of this, I think the workflow described above in How the hook works is excellent
  • make hook behavior (e.g. apply on fail) configurable
    • i prefer to have fewer config options, if someone needs it so much they're willing to write the PR and docs for it then I'm okay with adding them later
  • want this feature disabled by default and opt-in via plugin config
    • If I read the code correctly, spotlessInstallGitPrePushHook will only run if you explicitly call it - it's not a dependency of check or anything like that. Am I correct?
    • to me, that is "disabled by default" enough. If someone really wants it they can add it as a dependency of check or whatever.

This still needs docs in the README.md for each plugin.

@lazer-dev
Copy link
Author

lazer-dev commented Jul 17, 2025

If I read the code correctly, spotlessInstallGitPrePushHook will only run if you explicitly call it - it's not a dependency of check or anything like that. Am I correct?

Yes, that's correct — if you want this feature, you have to manually run the spotlessInstallGitPrePushHook command once.

to me, that is "disabled by default" enough. If someone really wants it they can add it as a dependency of check or whatever.

Yes, it's disabled by default — you're right. If someone wants it to run automatically, they can add it as a dependency to check, but I don't think we need to provide that functionality out of the box.

pre-commit hook has some problems (and a solution) outlined in #623

Yes, that's true. I'm not entirely sure the proposed solution works 100% reliably. In any case, that situation with pre-push happens quite rare, and I believe the shell script should stay as simple as possible. The edge case described in that issue doesn’t happen often enough to justify a more complex script that might introduce new bugs.

i prefer to have fewer config options, if someone needs it so much they're willing to write the PR and docs for it then

Hmm, if you really want this option, I’m happy to implement it. But just to consider — in 99.9999% of cases when spotlessCheck fails, the user will immediately run spotlessApply. Only in very rare cases does apply not fix everything and require manual edits. So in my opinion, the benefit of making this configurable is questionable. But again — if you want it, I’m happy to add it :)

This still needs docs in the README.md for each plugin.

Got it — could you please clarify what exactly is missing? Do you expect a short description of the hook task/goal, or also an example usage? Just want to make sure I include everything needed.

@lazer-dev lazer-dev force-pushed the git-pre-push-hook branch from a9205b6 to 6528b0b Compare July 17, 2025 04:00
@nedtwigg
Copy link
Member

docs ... clarify what exactly is missing

I just pushed up a commit with placeholders, your description at the open of the PR was a great start!

@lazer-dev lazer-dev force-pushed the git-pre-push-hook branch from b8ce688 to 525e70e Compare July 18, 2025 06:09
…h a pre-push hook or a pre-commit hook, depending on what the user prefers.
Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

I think this is excellent. I tweaked the docs to be shorter and to look ahead to a future where users have a choice between a pre-push hook or a pre-commit hook. If you're happy with it @lazer-dev, I'll merge and release.

@lazer-dev
Copy link
Author

I think this is excellent. I tweaked the docs to be shorter and to look ahead to a future where users have a choice between a pre-push hook or a pre-commit hook. If you're happy with it @lazer-dev, I'll merge and release.

Just one question — if a user manually modifies the git-push hook file and breaks the formatting, the uninstall logic might not work properly. Should we handle this case somehow?

@lazer-dev lazer-dev requested a review from nedtwigg July 18, 2025 20:39
@nedtwigg nedtwigg mentioned this pull request Jul 18, 2025
5 tasks
@lazer-dev lazer-dev force-pushed the git-pre-push-hook branch from 9fda21d to 957cb7a Compare July 19, 2025 04:02
@lazer-dev lazer-dev force-pushed the git-pre-push-hook branch from 957cb7a to 2dadeb6 Compare July 19, 2025 04:20
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.

3 participants