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

[Josh Steadmon <steadmon@google.com>]

On 2019.07.29 22:29, Martin Ågren wrote:
> On Fri, 19 Jul 2019 at 01:56, Josh Steadmon <steadmon@google.com> wrote:
> > 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.

"pre-merge-commit" sounds reasonable to me. I'll wait for a bit before
sending out V3 in case anyone else wants to chime in here though.

> 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