All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Michael J Gruber <git@drmicha.warpmail.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/3] git-merge: Honor pre-merge hook
Date: Wed, 05 Sep 2012 17:30:06 +0200	[thread overview]
Message-ID: <50476FFE.5070602@alum.mit.edu> (raw)
In-Reply-To: <dc8ebcd7f7b80ff930c04b5a407361ba8f2f077f.1346851863.git.git@drmicha.warpmail.net>

On 09/05/2012 03:39 PM, Michael J Gruber wrote:
> git-merge does not honor the pre-commit hook when doing automatic merge
> commits, and for compatibility reasons this is going to stay.
> 
> Introduce a pre-merge hook which is called for an automatic merge commit
> just like pre-commit is called for a non-automatic merge commit (or any
> other commit).

What exactly is an "automatic merge commit"?  Is it any merge that
doesn't have a conflict?  A merge that doesn't invoke the editor?  A
merge done as part of another operation (e.g., pull)?  I don't see the
term mentioned in the git-merge or githooks man pages.

I think it would be good if you would define this term in the
documentation files that your patch touched, and perhaps in the githooks
section about "pre-commit" as well.

Secondly, though it is impossible (for backwards compatibility reasons)
for the pre-commit hook to be invoked for automatic merges, no such
considerations prohibit the pre-merge commit from being invoked for
non-automatic merges.  In other words, both hooks, pre-commit *and*
pre-merge, could be invoked for non-automatic merges.  Would this be
preferable?

It depends on what pre-merge scripts are likely to be used for.  If they
will tend to be used for merge-specific actions, then it might be more
convenient for *all* merges to be vetted by them.  On the other hand, if
they tend to do the same actions as pre-commit hooks, then having
non-automatic merge commits go through both hooks would tend to be more
annoying than helpful.  Specifically, one of the scripts would probably
have to check whether the merge is a non-automatic merge, and if so do
nothing (i.e., letting the other script take care of it).  This would
also require an easy way for a script to determine whether a commit is a
non-automatic merge commit.

Have you considered this?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

  reply	other threads:[~2012-09-05 15:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-05 13:39 [PATCH 0/3] pre-merge-hook Michael J Gruber
2012-09-05 13:39 ` [PATCH 1/3] git-merge: Honor pre-merge hook Michael J Gruber
2012-09-05 15:30   ` Michael Haggerty [this message]
2012-09-06  8:16     ` Michael J Gruber
2012-09-06 10:48       ` Michael Haggerty
2012-09-06 14:25         ` [PATCHv2 0/4] pre-commit hook for merges Michael J Gruber
2012-09-06 14:25           ` [PATCHv2 1/4] merge: document prepare-commit-msg hook usage Michael J Gruber
2012-09-06 14:25           ` [PATCHv2 2/4] git-merge: Honor pre-commit hook based on config Michael J Gruber
2012-09-06 14:25           ` [PATCHv2 3/4] merge: --no-verify to bypass pre-commit hook Michael J Gruber
2012-09-06 14:25           ` [PATCHv2 4/4] t7503: add tests for pre-commit hook (merge) Michael J Gruber
2012-09-05 13:39 ` [PATCH 2/3] merge: --no-verify to bypass pre-merge hook Michael J Gruber
2012-09-05 13:39 ` [PATCH 3/3] t7503: add tests for pre-merge-hook Michael J Gruber
2012-09-06  5:07 ` [PATCH 0/3] pre-merge-hook Junio C Hamano
2012-09-06  8:16   ` Michael J Gruber
2012-09-06 18:34     ` Junio C Hamano
2012-09-07 10:00       ` Michael J Gruber

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50476FFE.5070602@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.