On Fri, 19 Jul 2019 at 01:56, Josh Steadmon <stead...@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. 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
