pre-commit hooks are awful, they get in the way of interactive rebasing. pre-push is where they belong; save a round-trip through CI.
This is what we have in our hooks:
if [ -d "$(git rev-parse --git-path rebase-merge)" ] || \
[ -d "$(git rev-parse --git-path rebase-apply)" ] || \
[ -f "$(git rev-parse --git-path MERGE_HEAD)" ]; then
exit 0
fiI don't think your argument is grounded on reality. Applying whitespace changes does create merge conflicts, and if you have a hook that is designed to introduce said white changes at each commit of a rebase them you are going to have frequent merge conflicts.
Keep also in mind that minor changes such as renaming a variable can and will introduce line breaks. Thus even with a pristine codebase that was formatted to perfection you will get merge conflicts.
> If any of the commits you are rebasing is e.g. breaking formatting rules, they shouldn't have been committed that way in the first place.
You're letting the world know you have little to no programming experience.
yes, formatters run on every commit. not only during rebase, but also every commit beforehand. if that is done consistently, the formatter does not cause merge conflicts.
merge conflicts during rebases due to variable name changes occur without commit hooks, too.
The hooks can fail my commit all they want, but I don't want them actually changing anything I've done, which definitely implies no reformatting in hooks.
Only if you rebase commits prior to the introduction of the hook. Otherwise that whitespace change should be already there in the old commits.
For formatting I find that it's clearly preferable to lean on the IDE and apply the source code formatter at each file save, and apply it only to the file you are touching. Type checks should be performed right before running unit tests, for the same reason unit tests are executed.
For type checking, I guess that makes sense if your unit tests are small and quick. At work type checking our entire codebase can take like 10-15 seconds with a cold cache, but running all unit tests takes 20 minutes (and multiple shards in CI). Seems like a no brainer to just run the cheap one more often.
This entire class of automation is awful and defeats the robustness of the tool itself.
All of these things have terribly unpredictable consequences and tend to fail at the worst moments, such as during a SEV.
You can encode the same rules and discipline in other ways that do not impact the health of the system, the quality of the data, or the ability of engineers to do work.