Subject: Re: [PATCH v5 11/15] bugreport: collect list of populated hooks

From: Emily Shaffer <[email-masked]>
Date: Tue, 4 Feb 2020 18:48:59 -0800

On Tue, Feb 04, 2020 at 10:44:39AM -0800, Junio C Hamano wrote:
> [email-masked] writes:
> 
> > +	/*
> > +	 * Doesn't look like there is a list of all possible hooks; so below is
> > +	 * a transcription of `git help hook`.
> 
> That's "git help hooks", if I tried my reproduction correctly.

Yep. I'll fix it.

> A straight-forward (in the sense of "what we want in the outcome is
> quite clear" and not in the sense of "anybody can design and implement
> it with a single 30-line patch") follow-up we can make after this
> series lands is to rethink how Documentation/githooks.txt is
> maintained and the list we have here is synchronized with it.
> 
> The design could me just the matter of running "grep" of some sort,
> with appropriate markups that are no-op to AsciiDoctor/AsciiDoc
> added to the documentation source, to produce this list.

Well, with Martin's suggestion[1] to use annotate:bugreport[include]
instead of bugreport:include[x], this kind of change becomes a matter of
course to just use "annotate:hook[x]" and write a script nearly
identical to generate-bugreport-config-safelist.sh, and add it to the
Makefile.

Like I said up-thread, I don't like the idea just because it's awfully
temporary (pending work that's right below this on my list for the
coming weeks).

It sounds like you're saying you don't mind any work of this nature
being done as a follow-up - and that's fine with me, too. I'd just
rather do the follow-up with enums and a library than with grep and
sh. :)

> >  static const char * const bugreport_usage[] = {
> >  	N_("git bugreport [-o|--output <file>]"),
> >  	NULL
> > @@ -193,6 +243,9 @@ int cmd_main(int argc, const char **argv)
> >  	get_header(&buffer, "Safelisted Config Info");
> >  	get_safelisted_config(&buffer);
> >  
> > +	get_header(&buffer, "Configured Hooks");
> 
> Phrase nit.  There may be many people who just enabled hooks without
> configuring, so "Enabled Hooks" may be more appropriate.  We do not
> have to inspect what is in the hook to determine if it is enabled,
> but we do need to if we want to tell if a hook is "configured".

Ah, that's a good point. OK, I'll change the wording as you suggested.

 - Emily