Re: [PATCH v7 3/4] advice: revamp advise API

[Junio C Hamano <gitster@pobox.com>]

"Heba Waly via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +static struct {
> +	const char *key;
> +	int enabled;
> +} advice_setting[] = {
> +	[ADVICE_ADD_EMBEDDED_REPO]			= { "addEmbeddedRepo", 1 },

It would be nicer to future developers to flip the polarity, as we
do not have to write 1 all over the place, especially if we plan to
extend the structure over time and to use designated initializers
for only certain fields:

	static struct {
		const char *key;
		int disabled;
	} advice_setting[] = {
		[ADDVICE_ADD_EMBEDDED_REPO] = { .key = "addEmbeddedRepo" },

> @@ -149,6 +218,13 @@ int git_default_advice_config(const char *var, const char *value)
>  		if (strcasecmp(k, advice_config[i].name))
>  			continue;
>  		*advice_config[i].preference = git_config_bool(var, value);
> +		break;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(advice_setting); i++) {
> +		if (strcasecmp(k, advice_setting[i].key))
> +			continue;
> +		advice_setting[i].enabled = git_config_bool(var, value);
>  		return 0;

Turning this into "break;" would make it similar to the loop before
this one, and will allow other people to add more code after this
loop later.

> +int cmd__advise_if_enabled(int argc, const char **argv)
> +{
> +	if (!argv[1])
> +	die("usage: %s <advice>", argv[0]);
> +
> +	setup_git_directory();
> +	git_config(git_default_config, NULL);
> +
> +	/*
> +	  Any advice type can be used for testing, but NESTED_TAG was selected
> +	  here and in t0018 where this command is being executed.
> +	 */

Style (will fix up locally).

Thanks.  I think this is reasonable with or without the suggested
fixes.