On Mon, Oct 05, 2020 at 04:24:18PM -0700, Jonathan Nieder wrote:
>
> Hi,
>
> Emily Shaffer wrote:
>
> > 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@xxxxxxxxxx>
> > ---
> > .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:
I would prefer to leave it on its own. Managing changes like
builtin<->standalone or even the one you mentioned below about
RUN_SETUP_GENTLY is somewhat easier to manage when they aren't in the
same patch as the business logic, IMO.
>
> [...]
> > --- /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.
Done.
>
> [...]
> > --- 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?
Nice catch. I'll add a test to the list patch to that effect also.
