Re: [PATCH] bugreport: collect list of populated hooks

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 25, 2020, 04:52 UTC
Message-ID: <xmqqsggs9ngo.fsf@gitster.c.googlers.com>
In-Reply-To: <20200424233800.200439-1-emilyshaffer@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 7 quoted lines

> Without a definitive list of hook names within the code, we compile our
> own list from the documentation. This is likely prone to bitrot. To
> reduce the amount of code humans need to read, we turn the list into a
> string_list and iterate over it (as we are calling the same find_hook
> operation on each string). However, since bugreport should primarily be
> called by the user, the performance loss from massaging the string
> seems acceptable.

In this iteration we no longer are collecting the hook names into string list, but just formating the findings in a strbuf, no?

Show 13 quoted lines

> @@ -33,6 +35,53 @@ static void get_system_info(struct strbuf *sys_info)
>  	get_libc_info(sys_info);
>  }
>  
> +static void get_populated_hooks(struct strbuf *hook_info, int nongit)
> +{
> +	/*
> +	 * NEEDSWORK: Doesn't look like there is a list of all possible hooks;
> +	 * so below is a transcription of `git help hooks`. Later, this should
> +	 * be replaced with some programmatically generated list (generated from
> +	 * doc or else taken from some library which tells us about all the
> +	 * hooks)
> +	 */

Yes, I recall that we discussed adding some annotation to documentation and extracting this automatically.

> +	const char *hook[] = {

Is it worth making this "static const"?

Show 7 quoted lines

> +	if (nongit) {
> +		strbuf_addstr(hook_info,
> +			"not run from a git repository - no hooks to show\n");
> +		return;
> +	}
> +
> +	for (i=0; i < ARRAY_SIZE(hook); i++)

Style: SP around = in "i=0".

> +	get_header(&buffer, "Enabled Hooks");
> +	get_populated_hooks(&buffer, nongit_ok);
> +

Sounds good, otherwise.