Re: [PATCH] sample pre-commit hook: don't trigger when recording a merge
Nanako Shiraishi <nanako3@lavabit.com> writes:
> I sent this patch but didn't receive any feedback. Is it a bad idea?
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.
> It may be better if 'git-commit' didn't call this hook when recording a merge,
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.
Show 36 quoted lines
>> When recording a merge, even if there are problematic whitespace errors,
>> let them pass, because the damage has already been done in the history. If
>> this hook triggers, it will invite a novice to fix such errors in a merge
>> commit but such a change is not related to the merge. Don't encourage it.
>>
>> Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
>> ---
>> templates/hooks--pre-commit.sample | 5 +++++
>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
>> index 439eefd..66e56bb 100755
>> --- a/templates/hooks--pre-commit.sample
>> +++ b/templates/hooks--pre-commit.sample
>> @@ -7,6 +7,11 @@
>> #
>> # To enable this hook, rename this file to "pre-commit".
>>
>> +if test -f "${GIT_DIR-.git}"/MERGE_HEAD
>> +then
>> + exit 0
>> +fi
>> +
>> if git-rev-parse --verify HEAD >/dev/null 2>&1
>> then
>> against=HEAD
>> --
>> 1.6.6
>>
>> --
>> Nanako Shiraishi
>> http://ivory.ap.teacup.com/nanako3/
>
> --
> Nanako Shiraishi
> http://ivory.ap.teacup.com/nanako3/
Nanako Shiraishi