Re: [PATCH v4 00/36] Run hooks via "git run hook" & hook library

On Tue, Aug 03, 2021 at 09:38:26PM +0200, Ævar Arnfjörð Bjarmason wrote:

This is a v4 re-roll of the "Base for "config-based-hooks" topic.

I performed the rebase of my series on top of this one, and did not find
very much objectionable there, but do not consider this email a thorough
review of the interdiff. I expect to have time for that later in the
week.

Other updates:

 * In the base topic the s/Signed-off-by/Reviewed-by/g from René
   change that Junio applied locally has been folded in.

 * Almost all the callers were just "one-shot" callers, I introduced a
   new run_hooks_oneshot() function for those, which gets rid of the
   verbosity around memory management, see e.g. the "builtin/gc.c" in
   the range-diff below. That run_hooks_oneshot() can also take a NULL
   set of options.

I am not so wild about this, to be honest, only because after my patch
2, it is hard to understand whether or not a hook is running in parallel
or in series. It is extra code-reader overhead to know that
run_hooks_oneshot() assumes that a hook can be parallelized, and I think
that is unfortunate.

But that is a pretty minor complaint, and I do like the lower
complexity, and I agree the defaults are "good enough" for most hook
events. So I guess I'm asking for ideas on how to make it more obvious
what this oneshot call does in terms of parallelism.

Thanks, and I hope to get to a fuller review of the interdiff soon.

 - Emily