Re: [PATCH v4 8/9] commit: use config-based hooks

> 
> > -	if (!no_verify && find_hook("pre-commit")) {
> > +	if (!no_verify && hook_exists("pre-commit")) {
> 
> A reviewer would probably need to look at all instances of "pre-commit"
> (and likewise for the other hooks) but if the plan is to convert all
> hooks, then the reviewer wouldn't need to do this since we could just
> delete the "find_hook" function.
> 
> Overall comments about the design and scope of the patch set:
> 
>  - I think that the abilities of the current patch set regarding
>    overriding order of globally-set hook commands is sufficient. We
>    should also have some way of disabling globally-set hooks, perhaps
>    by implementing the "skip" variable mentioned in patch 1 or by
>    allowing the redefinition of hookcmd sections (e.g. by redefining a
>    command to "/usr/bin/true"). To me, these provide substantial
>    user-facing value, and would be sufficient for a first version - and
>    other things like parallelization can come later.

> 
>  - As for the UI that should be exposed through the "git hook" command,
>    I think that "git hook list" and "git hook run" are sufficient.
>    Editing the config files are not too difficult, and "git hook add"
>    etc. can be added later.
> 
>  - As for whether (1) it is OK for none of the hooks to be converted (and
>    instead rely on the user to edit their hook scripts to call "git hook
>    run ???"), or if (2) we should require some hooks to be
>    converted, or if (3) we should require all hooks to be converted: I'd
>    rather have (2) or (3) so that we don't have dead code. I prefer (3),
>    especially since a reviewer wouldn't have to worry about leftover
>    usages of old functions like find_hook() (as I mentioned at the start
>    of this email), but I'm not fully opposed to (2) either.