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

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

 



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



[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]

  Powered by Linux