Re: [TOPIC 2/17] Hooks in the future

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Apr 15, 2020, 03:45 UTC
Message-ID: <20200415034550.GB36683@google.com>
In-Reply-To: <20200413215256.GA18990@coredump.intra.peff.net>

Hi,

Jeff King wrote:

> On Mon, Apr 13, 2020 at 12:15:15PM -0700, Emily Shaffer wrote:
>> Jeff King wrote:

Show 6 quoted lines

>>> Yeah, giving each block a unique name lets you give them each an order.
>>> It seems kind of weird to me that you'd define multiple hook types for a
>>> given name.
>>
>> Not so odd - git-secrets configures itself for pre-commit,
>> prepare-commit-msg, and commit-msg-hook.

[...]

> Yeah, I do see how that use case makes sense. I wonder how common it is
> versus having separate one-off hooks.

I think separately from the frequency question, we should look at the "what model do we want to present to the user" question.

It's not too unusual for a project with their source code in a Git repository to have conventions they want to nudge users toward. I'd expect them to use a combination of hooks for this:

	prepare-commit-msg
	commit-msg
	pre-push

Git LFS installs multiple hooks:

	pre-push
	post-checkout
	post-commit
	post-merge

git-secrets installs multiple hooks, as already mentioned.

We've also had some instances over time of one hook replacing another, to improve the interface. A program wanting to install hooks would then be likely to migrate from the older interface to the better one.

What I mean to get at is that I think thinking of them in terms of individual hooks, the user model assumed by these programs is to think of them as plugins hooking into Git. The individual hooks are events that the plugin listens on. If I am trying to disable a plugin, I don't want to have to learn which events it cared about.

>                                       And whether setting the order
> priority for all hooks at once is that useful (e.g., I can easily
> imagine a case where the pre-commit hook for program A must go before B,
> but it's the other way around for another hook).

This I agree about. Actually I'm skeptical about ordering dependencies being something that is meaningful for users to work with in general, except in the case of closely cooperating hook authors.

That doesn't mean we shouldn't try to futureproof for that, but I don't think we need to overfit on it.

[...]

>>> And it doesn't leave a lot of room for defining
>>> per-hook-type options; you have to make new keys like pre-push-order
>>> (though that does work because the hook names are a finite set that
>>> conforms to our config key names).

Exactly: field names like prePushOrder should work okay, even if
they're a bit noisy.

[...]

Show 8 quoted lines

>>>   [hook "pre-receive"]
>>>   # put any pre-receive related options here; e.g., a rule for what to
>>>   # do with hook exit codes (e.g., stop running, run all but return exit
>>>   # code, ignore failures, etc)
>>>   fail = stop
>>
>> Interesting - so this is a default for all pre-receive hooks, that I can
>> set at whichever scope I wish.

If I have the mental model of "these are plugins, and particular hooks are events they listen to", then it seems hard to make use of this broader setting.

But scoped to a particular (plugin, event) pair it sounds very handy.

My two cents, Jonathan