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

[Emily Shaffer <emilyshaffer@google.com>]

On Tue, Apr 14, 2020 at 08:45:50PM -0700, Jonathan Nieder wrote:
> 
> 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.

I find this argument particularly compelling :)

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

Striking out on finding another place to fit into the thread, I wonder
if the reason some of us are thinking "I'm going to write a pre-receive
hook" rather than "I'm going to write a linter hook" may be because of
the prior single-script-per-hook limitation. As a result, when you want
to add another function to your hook, you think, "I'll modify my
pre-receive hook". I think part of this RFC is a subtle paradigm shift
away from hooks-as-units-of-work and towards hooks-as-events.

That observation doesn't really provide much guidance though, except
maybe to point out we should think about what the glossary entries would
say for terms like "hook" and "hook command" now... and I think figuring
out those definitions might help us settle on what is most logical in
the config.

(That makes me think I had better write a design doc next, before I get
too much further with RFC patches. I made one pass at one a while ago,
but it was more focused on history and choosing between alternatives;
since we seem to have agreed on an approach, I'll make another attempt
focusing on design and definition instead. I'll try to have something to
the list by next week.)

 - Emily