Re: [PATCH v4 2/9] hook: scaffolding for git-hook subcommand

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 5, 2020, 23:24 UTC
Message-ID: <20201005232418.GA1393696@google.com>
In-Reply-To: <20200909004939.1942347-3-emilyshaffer@google.com>

Hi,

Emily Shaffer wrote:

Show 18 quoted lines

> Introduce infrastructure for a new subcommand, git-hook, which will be
> used to ease config-based hook management. This command will handle
> parsing configs to compose a list of hooks to run for a given event, as
> well as adding or modifying hook configs in an interactive fashion.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>  .gitignore                    |  1 +
>  Documentation/git-hook.txt    | 19 +++++++++++++++++++
>  Makefile                      |  1 +
>  builtin.h                     |  1 +
>  builtin/hook.c                | 21 +++++++++++++++++++++
>  git.c                         |  1 +
>  t/t1360-config-based-hooks.sh | 11 +++++++++++
>  7 files changed, 55 insertions(+)
>  create mode 100644 Documentation/git-hook.txt
>  create mode 100644 builtin/hook.c
>  create mode 100755 t/t1360-config-based-hooks.sh

optional: I could imagine this being squashed into patch 3 --- that way, the command has functionality as soon as it exists. Alternatively:

[...]

Show 18 quoted lines

> --- /dev/null
> +++ b/Documentation/git-hook.txt
> @@ -0,0 +1,19 @@
> +git-hook(1)
> +===========
> +
> +NAME
> +----
> +git-hook - Manage configured hooks
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git hook'
> +
> +DESCRIPTION
> +-----------
> +You can list, add, and modify hooks with this command.

This could say something like "This is a placeholder command that will gain functionality in subsequent patches" to make the current state clear.

[...]

Show 7 quoted lines

> --- a/git.c
> +++ b/git.c
> @@ -519,6 +519,7 @@ static struct cmd_struct commands[] = {
>  	{ "grep", cmd_grep, RUN_SETUP_GENTLY },
>  	{ "hash-object", cmd_hash_object },
>  	{ "help", cmd_help },
> +	{ "hook", cmd_hook, RUN_SETUP },

This makes the command require that it run within a git repository, but I can imagine wanting to list hooks outside of any. Should it use RUN_SETUP_GENTLY instead?

Thanks, Jonathan