Re: [PATCH v2 0/4] pre-merge hook

From: Martin Ågren <martin.agren@gmail.com>
Date: Jul 29, 2019, 20:29 UTC
Message-ID: <CAN0heSr4N-+fjn3+58oUvvv57vDXtA2tR-NEEyoS4toj36V2-Q@mail.gmail.com>
In-Reply-To: <cover.1563490164.git.steadmon@google.com>

On Fri, 19 Jul 2019 at 01:56, Josh Steadmon <steadmon@google.com> wrote:

Show 6 quoted lines

> This series revives an old suggestion [...] to make merge honor
> pre-commit hook or a separate pre-merge hook. Since pre-commit does not
> receive any arguments (which the hook could use tell between commit and
> merge) and in order to keep existing behaviour (as opposed to the other
> patch) this series introduces a pre-merge hook, with a sample hook that
> simply calls the pre-commit hook.

Argh, I wanted to comment on this in the cover letter, but forgot and just left a bunch of nits.

I wonder if we might ever regret naming this "pre-merge" and not, e.g., "pre-merge-commit". There's the pre-rebase hook which runs before git-rebase even starts actually rebasing, so I could well imagine a "pre-merge" hook which would get called before merging starts.

I'm also pondering whether there should be an "automatic" in there, but "pre-automatic-merge-commit" looks a bit awkward. Anyway, I'm not even sure an "automatic merge commit" is a well-defined thing. I can figure out what it pretty certainly means, but it's not crystal clear. There might be a need for some more discussion in the added docs for what this new hook is for and how it compares to pre-commit. Right now, the proposed docs suggests they're equivalent in a way, but I think that's a bit confusing -- they are certainly not synonyms, and they'd never both be called, for example. They can be used for the same purpose in different scenarios, sure.

I tried coming up with some proposed docs for githooks.txt, but didn't feel I achieved much of value. :-/

Martin