Re: [PATCH] merge: Run commit-msg hook

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Jul 26, 2016 at 4:02 PM, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
> Hi Orgad,
>
> On Tue, 26 Jul 2016, Orgad Shaneh wrote:
>
>> From: Orgad Shaneh <orgads@xxxxxxxxx>
>
> Again, this is unnecessary if you already send the mail from the same
> address.
>
>> commit-msg is needed to either validate the commit message or edit it.
>> Gerrit for instance uses this hook to append its Change-Id footer.
>>
>> This is relevant to merge commit just like any other commit.
>
> Hmm. This is not very convincing to me, as
>
> - if you call commit-msg in `git merge` now, why not `prepare-commit-msg`?

prepare-commit-msg is already called, a few lines above this addition.

>
> - a merge is a different beast from a simple commit. That is why we have
>   two different commands for them. A hook to edit the merge message may
>   need to know the *second* parent commit, too, for example to generate
>   a diffstat, or to add information about changes in an "evil commit".

That is correct for a post-merge hook. Why should *message validation* differ
between simple and merge commit?

> - if Gerrit is the intended user, would it not make more sense to
>   introduce a new hook, e.g. `merge-msg` (and `prepare-merge-msg`), as you
>   have to teach Gerrit a new trick anyway?

Why is that new? Every commit in gerrit has a Change-Id footer, which is
generated by commit-msg hook. What I currently do for merges that succeed
without conflicts is unconditional commit --amend --no-edit just to
run the hook.
This doesn't make sense.

> - if Gerrit is the intended user, why does it not simply edit the merge
>   message itself? After all, it executes it, and probably crafts a merge
>   message mentioning that this is an automatic merge, anyway, so why not
>   add the Change-Id *then*?

Most Gerrit setups require Change-Id in the commit message that the user
pushes. It is possible to disable this setting, and then you don't need the
Change-Id at all, but then you can't push a new patch set for the change
(unless you copy the Change-Id from gerrit to your commit message).
That's a real pain, I'm not aware of any public gerrit server that
disables this :)

> Ciao,
> Dscho

- Orgad
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]