Re: [PATCH] sample pre-commit hook: don't trigger when recording a merge

Nanako Shiraishi <nanako3@lavabit.com> writes:

I think the behaviour might make sense for some workflows but not always, and it also depends on what is checked by the hook, and the extent of damage detected by the check for a particular merge.

The sample hook checks whitespace breakage, and in the context of merging other people's branch to obtain a change that is huge enough that neither I nor the original author may be inclined to fix it up, it _might_ sense to say that (1) the other branch is already borked and (2) it is too late to fix it up.

But these two are different from (3) there is not much point complaining.

I doubt it. I think it largely depends on where you are merging to as well. I may respond to a pull request by others by merging to 'pu', to give the topic wider visibility, and in such a case I may wish to disable the check. On the other hand, when I redo the merge to advance the same topic to 'next', I may want to notice the breakage so that I can tell the person to clean up the branch before asking me to pull for real.

So I am slightly negative on this change; it would be better to reject committing the merge and let the user decide if it is too much to bother fixing up the other branch (in which case you would reset the merge away), or let small breakages pass by running "git commit --no-verify" to bypass the hook explicitly.