From: Emily Shaffer <emilyshaffer@google.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
"Jeff Hostetler" <jeffhost@microsoft.com>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
"Felipe Contreras" <felipe.contreras@gmail.com>,
"SZEDER Gábor" <szeder.dev@gmail.com>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"René Scharfe" <l.s.r@web.de>
Subject: Re: [PATCH v2 3/3] hook-list.h: add a generated list of hooks, like config-list.h
Date: Mon, 12 Jul 2021 13:55:31 -0700 [thread overview]
Message-ID: <YOysQ7ewkpLZMP2I@google.com> (raw)
In-Reply-To: <87wnpy4jyb.fsf@evledraar.gmail.com>
On Sat, Jul 10, 2021 at 11:03:50AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>
> On Fri, Jul 09 2021, Emily Shaffer wrote:
>
> > On Tue, Jun 29, 2021 at 08:54:02PM +0200, Ævar Arnfjörð Bjarmason wrote:
> >> const char *find_hook(const char *name)
> >> {
> >> static struct strbuf path = STRBUF_INIT;
> >>
> >> + if (!known_hook(name))
> >> + die(_("the hook '%s' is not known to git, should be in hook-list.h via githooks(5)"),
> >> + name);
> >> +
> >
> > I'm not sure that it's necessary to require this, to be honest. I see a
> > use case for wrappers to want to store and run hooks in an idiomatic
> > way, and doing so by instructing their users to stick in
> > .git/hooks/wrapper-clone (for example) and then calling 'git hook run
> > wrapper-clone'. That's doubly compelling in a later config-based-hooks
> > world where 'git hook run' gets you free multihook features like
> > ordering and parallelism. I will likely want to remove this when
> > rebasing my config-based hooks work on top of your restart.
>
> Indeed, FWIW this was part of my general approach of narrowly supporting
> existing git behavior only with 'git hook run', i.e. there's no general
> "run this thing like a hook for me" now, so we're not losing anything by
> not having it support that.
>
> But yes, I can see how "run this script for me as if though it were a
> hook" would be useful, will be trivial to support it & still somehow
> assert typos/that hook-list.h / githooks.txt is a source of truth about
> our known hooks.
Hm, I see - you're using the BUG() to gently remind people that they
should go and update githooks.txt if they call find_hook("new-hook").
Ok, I'll add a flag or a wrapper or something in my own rebase - so 'git
hook run' doesn't check but internal calls do.
Thanks for explaining the intent better.
- Emily
prev parent reply other threads:[~2021-07-12 20:55 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-17 10:09 [PATCH 0/3] Add a generated list of hooks in hook-list.h Ævar Arnfjörð Bjarmason
2021-06-17 10:09 ` [PATCH 1/3] hook.[ch]: move find_hook() to this new library Ævar Arnfjörð Bjarmason
2021-06-17 10:09 ` [PATCH 2/3] hook.c: add a hook_exists() wrapper and use it in bugreport.c Ævar Arnfjörð Bjarmason
2021-06-17 10:09 ` [PATCH 3/3] hook-list.h: add a generated list of hooks, like config-list.h Ævar Arnfjörð Bjarmason
2021-06-18 17:05 ` SZEDER Gábor
2021-06-18 17:50 ` Eric Sunshine
2021-06-19 6:06 ` Junio C Hamano
2021-06-20 13:53 ` Ævar Arnfjörð Bjarmason
2021-06-20 12:53 ` René Scharfe
2021-06-22 22:32 ` Johannes Schindelin
2021-06-29 0:32 ` Junio C Hamano
2021-06-29 17:53 ` René Scharfe
2021-06-29 18:53 ` [PATCH v2 0/3] Add a generated list of hooks in hook-list.h Ævar Arnfjörð Bjarmason
2021-06-29 18:54 ` [PATCH v2 1/3] hook.[ch]: move find_hook() to this new library Ævar Arnfjörð Bjarmason
2021-06-29 18:54 ` [PATCH v2 2/3] hook.c: add a hook_exists() wrapper and use it in bugreport.c Ævar Arnfjörð Bjarmason
2021-06-29 18:54 ` [PATCH v2 3/3] hook-list.h: add a generated list of hooks, like config-list.h Ævar Arnfjörð Bjarmason
2021-06-29 19:45 ` René Scharfe
2021-06-29 22:09 ` Ævar Arnfjörð Bjarmason
2021-07-09 20:29 ` Emily Shaffer
2021-07-10 9:03 ` Ævar Arnfjörð Bjarmason
2021-07-12 20:55 ` Emily Shaffer [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YOysQ7ewkpLZMP2I@google.com \
--to=emilyshaffer@google.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=avarab@gmail.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jeffhost@microsoft.com \
--cc=l.s.r@web.de \
--cc=sunshine@sunshineco.com \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.