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

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

[Nanako Shiraishi]

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/

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

[Nanako Shiraishi]

Hi,

I sent this patch but didn't receive any feedback. Is it a bad idea?
It may be better if 'git-commit' didn't call this hook when recording a merge,
but I don't write C very much.

Thanks.

> 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/

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

[Junio C Hamano]

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.

>> 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/