{"thread":{"id":"22139","subject":"[PATCH] sample pre-commit hook: don't trigger when recording a merge","startedAt":"2010-01-08T23:16:52Z","lastAt":"2010-01-17T22:24:49Z","messageCount":3,"participants":["Nanako Shiraishi","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"131144","messageId":"20100109081652.6117@nanako3.lavabit.com","threadId":"22139","inReplyTo":null,"subject":"[PATCH] sample pre-commit hook: don't trigger when recording a merge","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2010-01-08T23:16:52Z","receivedAt":"2010-01-08T23:16:52Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"When recording a merge, even if there are problematic whitespace errors,\nlet them pass, because the damage has already been done in the history. If\nthis hook triggers, it will invite a novice to fix such errors in a merge\ncommit but such a change is not related to the merge. Don't encourage it.\n\nSigned-off-by: Nanako Shiraishi <nanako3@lavabit.com>\n---\n templates/hooks--pre-commit.sample |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample\nindex 439eefd..66e56bb 100755\n--- a/templates/hooks--pre-commit.sample\n+++ b/templates/hooks--pre-commit.sample\n@@ -7,6 +7,11 @@\n #\n # To enable this hook, rename this file to \"pre-commit\".\n \n+if test -f \"${GIT_DIR-.git}\"/MERGE_HEAD\n+then\n+\texit 0\n+fi\n+\n if git-rev-parse --verify HEAD >/dev/null 2>&1\n then\n \tagainst=HEAD\n-- \n1.6.6\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"131990","messageId":"20100118061636.6117@nanako3.lavabit.com","threadId":"22139","inReplyTo":"20100109081652.6117@nanako3.lavabit.com","subject":"Re: [PATCH] sample pre-commit hook: don't trigger when recording a merge","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2010-01-17T21:16:36Z","receivedAt":"2010-01-17T21:16:36Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Hi,\n\nI sent this patch but didn't receive any feedback. Is it a bad idea?\nIt may be better if 'git-commit' didn't call this hook when recording a merge,\nbut I don't write C very much.\n\nThanks.\n\n> When recording a merge, even if there are problematic whitespace errors,\n> let them pass, because the damage has already been done in the history. If\n> this hook triggers, it will invite a novice to fix such errors in a merge\n> commit but such a change is not related to the merge. Don't encourage it.\n>\n> Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>\n> ---\n>  templates/hooks--pre-commit.sample |    5 +++++\n>  1 files changed, 5 insertions(+), 0 deletions(-)\n>\n> diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample\n> index 439eefd..66e56bb 100755\n> --- a/templates/hooks--pre-commit.sample\n> +++ b/templates/hooks--pre-commit.sample\n> @@ -7,6 +7,11 @@\n>  #\n>  # To enable this hook, rename this file to \"pre-commit\".\n>  \n> +if test -f \"${GIT_DIR-.git}\"/MERGE_HEAD\n> +then\n> +\texit 0\n> +fi\n> +\n>  if git-rev-parse --verify HEAD >/dev/null 2>&1\n>  then\n>  \tagainst=HEAD\n> -- \n> 1.6.6\n>\n> -- \n> Nanako Shiraishi\n> http://ivory.ap.teacup.com/nanako3/\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"131992","messageId":"7vd418xtce.fsf@alter.siamese.dyndns.org","threadId":"22139","inReplyTo":"20100118061636.6117@nanako3.lavabit.com","subject":"Re: [PATCH] sample pre-commit hook: don't trigger when recording a merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-17T22:24:49Z","receivedAt":"2010-01-17T22:24:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> I sent this patch but didn't receive any feedback. Is it a bad idea?\n\nI think the behaviour might make sense for some workflows but not always,\nand it also depends on what is checked by the hook, and the extent of\ndamage detected by the check for a particular merge.\n\nThe sample hook checks whitespace breakage, and in the context of merging\nother people's branch to obtain a change that is huge enough that neither\nI nor the original author may be inclined to fix it up, it _might_ sense\nto say that (1) the other branch is already borked and (2) it is too late\nto fix it up.\n\nBut these two are different from (3) there is not much point complaining.\n\n> It may be better if 'git-commit' didn't call this hook when recording a merge,\n\nI doubt it.  I think it largely depends on where you are merging to as\nwell.  I may respond to a pull request by others by merging to 'pu', to\ngive the topic wider visibility, and in such a case I may wish to disable\nthe check.  On the other hand, when I redo the merge to advance the same\ntopic to 'next', I may want to notice the breakage so that I can tell the\nperson to clean up the branch before asking me to pull for real.\n\nSo I am slightly negative on this change; it would be better to reject\ncommitting the merge and let the user decide if it is too much to bother\nfixing up the other branch (in which case you would reset the merge away),\nor let small breakages pass by running \"git commit --no-verify\" to bypass\nthe hook explicitly.\n\n>> When recording a merge, even if there are problematic whitespace errors,\n>> let them pass, because the damage has already been done in the history. If\n>> this hook triggers, it will invite a novice to fix such errors in a merge\n>> commit but such a change is not related to the merge. Don't encourage it.\n>>\n>> Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>\n>> ---\n>>  templates/hooks--pre-commit.sample |    5 +++++\n>>  1 files changed, 5 insertions(+), 0 deletions(-)\n>>\n>> diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample\n>> index 439eefd..66e56bb 100755\n>> --- a/templates/hooks--pre-commit.sample\n>> +++ b/templates/hooks--pre-commit.sample\n>> @@ -7,6 +7,11 @@\n>>  #\n>>  # To enable this hook, rename this file to \"pre-commit\".\n>>  \n>> +if test -f \"${GIT_DIR-.git}\"/MERGE_HEAD\n>> +then\n>> +\texit 0\n>> +fi\n>> +\n>>  if git-rev-parse --verify HEAD >/dev/null 2>&1\n>>  then\n>>  \tagainst=HEAD\n>> -- \n>> 1.6.6\n>>\n>> -- \n>> Nanako Shiraishi\n>> http://ivory.ap.teacup.com/nanako3/\n>\n> -- \n> Nanako Shiraishi\n> http://ivory.ap.teacup.com/nanako3/\n"}]}