Discussion of the Benefits and Drawbacks of the Git Pre-Commit Hook

16 pointsposted 11 days ago
by hambes

12 Comments

lemagedurage

an hour ago

Pre commit hooks shine with fast formatters. Keep the hook under half a second or so and it's great.

0123456789ABCDE

38 minutes ago

it's the tools. if they're slow people disable them or shift them right. you want your defect-detection and fixing to shift left

if you're not running auto-format on file-save, your auto-formatter is slow

if you're not running a code checker with auto-fix on pre-commit, your code checker is slow

if you're not running the test-suite on pre-push your tests are slow

if your tooling is slow you need to pick better tooling or make them fast

you want to keep that loop tight and active

ozzydave

3 hours ago

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.

jakub_g

19 minutes ago

You can check inside the hook if you're in the middle of the rebase, and exit the hook early.

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
    fi

extr

an hour ago

Yeah there's really no trouble with a pre-push hook that runs common/fast code checks. For TS projects I just run formatting, type, and lint checks. Faster feedback than spinning up a runner in CI and if I don't need it I just tack on --no-verify.

motorest

11 minutes ago

> For TS projects I just run formatting, type, and lint checks.

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.

hambes

an hour ago

I would argue that if the pre-commit hooks come in the way of rebasing, either the commit hooks are doing way too much (which is one of the points of the article) or you are creating broken commits during rebasing. 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.

motorest

18 minutes ago

> I would argue that if the pre-commit hooks come in the way of rebasing, either the commit hooks are doing way too much (which is one of the points of the article) or you are creating broken commits during rebasing.

I 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.

hambes

10 minutes ago

maybe, because i really don't have the problem you're describing.

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.

echelon

2 hours ago

Database integrity constraints fall into the same category.

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.

esafak

3 hours ago

agents trip up over them too.

fphilipe

2 hours ago

I'm of the opinion that Git hooks are personal. That's why they're not part of the source code itself. I make extensive use of hooks, but they're tailored to my needs.

Note that you can skip hooks by passing the --no-verify flag to subcommands. Comes in handy when they're slow and you know that you've just fixed the wrong formatting that the previous invocation of your pre-commit hook complained about.