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

From: Emily Shaffer <[email-masked]>
Date: Tue, 7 Apr 2020 16:51:16 -0700

On Tue, Apr 07, 2020 at 04:01:32PM -0700, Emily Shaffer wrote:
> Thoughts?

Jonathan Nieder and I discussed this a little bit offline, and he
suggested another thought:

[hook "unique-name"]
  pre-commit = ~/path-to-hook.sh args-for-precommit
  pre-push = ~/path-to-hook.sh
  order = 001

Then, in another config:

hook.unique-name.pre-push-order = 123

or,

hook.unique-name.enable = false
hook.unique-name.pre-commit-enable = true

To pick it apart a little more:

 - Let's give each logical action a unique name, e.g. "git-secrets".
 - Users can sign up for a certain event by providing the command to
   run, e.g. `hook.git-secrets.pre-commit = git-secrets pre-commit`.
 - Users can set up defaults for the logical action, e.g.
   `hook.git-secrets.before = gerrit` (where "gerrit" is the unique name
   for another logical action), and then change it on a per-hook basis
   e.g. `hook.git-secrets.pre-commit-before = clang-tidy`

There's some benefit:
 - We don't have to kludge something new (multiple sections with the
   same name, but logically disparate) into the config semantics where
   it doesn't really fit.
 - Users could, for example, turn off all "git-secrets" invocations in a
   repo without knowing which hooks it's attached to, e.g.
   `hook.git-secrets.enable = false`
 - We still have the option to add and remove parameters like 'order' or
   'before'/'after' or 'parallelizable' or etc., on a per-hook basis or
   for all flavors of a logical action such as "git-secrets"
 - It may be easier for a config-authoring iteration of 'git-hook' to
   modify existing configs than it would be if the ordering of config
   entries is vital.

One drawback I can think of is that these unique names could be either
difficult to autogenerate and guarantee uniqueness, or difficult for
humans to parse. I'd have to rethink the UI for writing or editing with
git-hook (rather than editing the config by hand), although I think with
the mood shifting away from configs looking like
"hook.pre-commit=123:~/path-to-thing.sh" my UI mockups are all invalid
anyways :)

We also considered something like:

[hook "git-secrets-pre-commit"]
  command = ~/path-to-hooks.sh args-for-precommit
  order = 001

[hook "git-secrets-pre-push"]
  comand = ~/path-to-hook.sh
  order = 123

but concluded that it's more verbose without adding additional value
over the earlier proposal above. This syntax can achieve a subset of
goals but is missing extra value like an easy path to disable all hooks
in that logical action, or nice defaults when you don't expect the order
or parallelism to change.

Definitely interested in hearing more ideas :)

 - Emily