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.

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.

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!

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?

@nedtwigg @cmoine-swi

Guys, if there are no further comments, I think we can merge?

Published in plugin-gradle 7.2.0 and plugin-maven 2.46.0

Hello !
There is an issue with the pre-push hook logic: when maven wrapper is used, it always goes for the mvnw file, even on Windows. But for Windows users, the mvnw file doesn't work (at least, it doesn't with powershell or cmd), we need to use mvnw.cmd.

I tried to used the pre-push hook maven goal, but when using Windows, the pre-push hook fails. 😥

In Mac, there are also problems. The code below would return the relative path from the root of the project, not the absolute path. Using a git client like Fork, for example, pushing will run into problems since it can't find the executor.

if (mvnw.exists()) {
  return mvnw.getAbsolutePath();
}

Maybe adding a configuration property would be a better approach

<execution>
    <goals>
        <goal>install-git-pre-push-hook</goal>
    </goals>
    <phase>compile</phase>
    <configuration>
        <executor>mvn</executor>
    </configuration>
</execution>

Hello ! There is an issue with the pre-push hook logic: when maven wrapper is used, it always goes for the mvnw file, even on Windows. But for Windows users, the mvnw file doesn't work (at least, it doesn't with powershell or cmd), we need to use mvnw.cmd.

I tried to used the pre-push hook maven goal, but when using Windows, the pre-push hook fails. 😥

@PierreLouisBertrand What version of Spotless are you using? I recommend upgrading to the latest version, which now supports .cmd and .bat files.

In Mac, there are also problems. The code below would return the relative path from the root of the project, not the absolute path. Using a git client like Fork, for example, pushing will run into problems since it can't find the executor.

if (mvnw.exists()) {
  return mvnw.getAbsolutePath();
}

Maybe adding a configuration property would be a better approach

<execution>
    <goals>
        <goal>install-git-pre-push-hook</goal>
    </goals>
    <phase>compile</phase>
    <configuration>
        <executor>mvn</executor>
    </configuration>
</execution>

@MarcoMartins86 added absolute path here #2701