Re: [PATCH v4 2/9] hook: scaffolding for git-hook subcommand
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Oct 6, 2020, 19:06 UTC
- Message-ID
- <20201006190616.GI331156@google.com>
- In-Reply-To
- <20201005232418.GA1393696@google.com>
On Mon, Oct 05, 2020 at 04:24:18PM -0700, Jonathan Nieder wrote:
Show 26 quoted lines
> > 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@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:
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.
Show 24 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.
Done.
Show 13 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?Nice catch. I'll add a test to the list patch to that effect also.