From: Derrick Stolee <derrickstolee@github.com>
Date: Thu, 02 Jun 2022 13:15:36 GMT
Subject: Re: [PATCH v3 4/5] config: include "-c" in protected config
Message-ID: <c9f11902-31aa-6b69-f4fc-06ed66887de1@github.com>
In-Reply-To: <66a0a208176094415d7b0ae1f686bdd1638b8a8a.1653685761.git.gitgitgadget@gmail.com>

On 5/27/2022 5:09 PM, Glen Choo via GitGitGadget wrote:
  
> +int git_configset_add_parameters(struct config_set *cs)
> +{
> +	return git_config_from_parameters(config_set_callback, cs);
> +}
> +

This one-line method could be inlined into the read_protected_config()
method:

> @@ -2628,6 +2633,7 @@ static void read_protected_config(void)
>  	git_configset_add_file(the_repository->protected_config, system_config);
>  	git_configset_add_file(the_repository->protected_config, xdg_config);
>  	git_configset_add_file(the_repository->protected_config, user_config);
> +	git_configset_add_parameters(the_repository->protected_config);
	git_config_from_parameters(config_set_callback, the_repository->protected_config);

...would be the way to inline it.

> +/**
> + * Parses command line options and environment variables, and adds the
> + * variable-value pairs to the `config_set`. Returns 0 on success, or -1
> + * if there is an error in parsing. The caller decides whether to free
> + * the incomplete configset or continue using it when the function
> + * returns -1.
> + */
> +int git_configset_add_parameters(struct config_set *cs);

You do make it public here. I wonder if we can think of other consumers
of this method that justify the addition to the API.

But this is also a nitpick. I don't feel strongly one way or another. The
code definitely works as-is.

Thanks,
-Stolee