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

From: Emily Shaffer <[email-masked]>
Date: Wed, 18 Aug 2021 17:17:49 -0700

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