Add pre-commit hooks script #43656
Add pre-commit hooks script #43656
Conversation
srnagar
requested review from
hallipr,
weshaggard,
benbp,
JimSuplizio,
joshfree,
anuchandy,
conniey,
lmolkova and
JonathanGiles
as code owners
|
/check-enforcer override |
| @@ -0,0 +1,36 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
@srnagar can you give more context on this? In general we have avoided git hooks because they are very fragile and cannot be relied on. From all our other trials they have caused more problems than they have helped.
There was a problem hiding this comment.
This was a MQ task. More details on this can be found in this issue - #41715
The main reason was to ensure we did some basic validation like cred scanner and spell checks done before the changes are committed as these are more sensitive and don't want it to be part of commit history. This PR only contains a script and doesn't automatically enable pre-commit hooks for everyone. It is an opt-in feature.
Could you please let me know what were some of the problems this caused in the past?
There was a problem hiding this comment.
There are a number of issues:
- There is no way to automatically setup a git hook so they cannot be relied on because they may not be enabled.
- Running random tools during this pre-commit time cause random failures. Anytime a failure occurs in one of these tools the commits also fail. This leads to often needing to explicitly disabling the hooks do to get a commit in.
We have tried to use these in .NET and JS repos in the past and they both decided to revert using them because of a lot of issues. I also have had nothing but issues with these in my past teams as well which is why I don't recommend them.
There was a problem hiding this comment.
Just to close the loop on this, we discussed this in the Java team meeting and will keep the script (with some improvements) in the eng/scripts directory. This will not automatically be used and will allow users of the repo to opt in to using this, if needed. If we run into issues, it's easy to opt out.
Fixes #41715