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

From: Jonathan Nieder <[email-masked]>
Date: Tue, 14 Apr 2020 20:45:50 -0700

Hi,

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

>>> 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.

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