Re: [RFC/PATCH 2/2] config: Add new query functions to the api

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 28, 2014, 09:43 UTC
Message-ID: <CAPig+cQZMNSZ-3C0zR3mvxp0n7C_L79wQex5L85Xr_w+8yvBZA@mail.gmail.com>
In-Reply-To: <1401125601-18249-3-git-send-email-tanayabh@gmail.com>

On Mon, May 26, 2014 at 1:33 PM, Tanay Abhra <tanayabh@gmail.com> wrote:

> Subject: config: Add new query functions to the api

This sounds as if you are adding new functions to the header file. Saying "... to the api docs" would been clearer. Better:

    config: document new query functions

> Add explanations for `git_config_get_string_multi` and `git_config_get_string`
> which utilize the config cache for querying in an non callback manner for a

s/an/a/ s/non/non-/

> specific variable.
>
> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>

One might expect these functions to be documented in the same patch which introduces them. Since this patch is so small, it might make sense just to squash it into patch 1.

More below.

Show 14 quoted lines

> ---
>  Documentation/technical/api-config.txt | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt
> index 230b3a0..33b5b90 100644
> --- a/Documentation/technical/api-config.txt
> +++ b/Documentation/technical/api-config.txt
> @@ -77,6 +77,25 @@ To read a specific file in git-config format, use
>  `git_config_from_file`. This takes the same callback and data parameters
>  as `git_config`.
>
> +Querying for specific variables
> +-------------------------------

All other section headers in this file capitalize each word.

> +For programs wanting to query for specific variables in a non callback

s/non-/

Show 7 quoted lines

> +manner, the config API provides two functions `git_config_get_string`
> +and `git_config_get_string_multi`. They both take a single parameter,
> +
> +- a variable as the key string for which the corresponding values will
> +  be retrieved and returned.
> +
> +They both read value from an internal cache generated previously from

Minor observation: As this file is documenting the API, mentioning private implementation details such, as the "internal cache", may be frowned upon (though probably doesn't matter in practice since this _is_ a technical document).

Show 13 quoted lines

> +reading the config files. `git_config_get_string` returns the value with
> +the highest priority(i.e. value in the repo config will be preferred over
> +value in user wide config for the same variable).
> +
> +`git_config_get_string_multi` returns a `string_list` containing all the
> +values for that particular variable, sorted in order of increasing
> +priority.
> +
>  Value Parsing Helpers
>  ---------------------
>
> --
> 1.9.0.GIT