Re: [PATCH 1/1] config: add documentation to config.h

From: Jonathan Tan <jonathantanmy@google.com>
Date: Oct 18, 2019, 22:07 UTC
Message-ID: <20191018220705.241778-1-jonathantanmy@google.com>
In-Reply-To: <2e42eafb5db6192829e9e206e9e9905b31f8e8a6.1571357219.git.gitgitgadget@gmail.com>

> From: Heba Waly <heba.waly@gmail.com>
> 
> This commit is copying and summarizing the documentation from
> documentation/technical/api-config.txt to comments in config.h

Thanks for this commit!

As for your commit message, as far as I know, the idea is to move the documentation, not to copy it. Also, write this in imperative form, e.g.:

  Move the documentation from Documentation/technical/api-config.txt
  into config.h.

Also change the title of the commit message accordingly, e.g.:

  config: move documentation to header file

Also, include the deletion of api-config.txt in this commit.

If you are doing any summarizing, describe what summarizing you are doing in the commit message too.

Show 25 quoted lines

> + * A config callback function takes three parameters:
> + *
> + * - the name of the parsed variable. This is in canonical "flat" form: the
> + *   section, subsection, and variable segments will be separated by dots,
> + *   and the section and variable segments will be all lowercase. E.g.,
> + *   `core.ignorecase`, `diff.SomeType.textconv`.
> + *
> + * - the value of the found variable, as a string. If the variable had no
> + *   value specified, the value will be NULL (typically this means it
> + *   should be interpreted as boolean true).
> + *
> + * - a void pointer passed in by the caller of the config API; this can
> + *   contain callback-specific data
> + *
> + * A config callback should return 0 for success, or -1 if the variable
> + * could not be parsed properly.
> + */
> +
>  struct object_id;
>  
>  /* git_config_parse_key() returns these negated: */
> @@ -73,6 +107,11 @@ struct config_options {
>  
>  typedef int (*config_fn_t)(const char *, const char *, void *);
>  int git_default_config(const char *, const char *, void *);

The config callback is config_fn_t so that documentation should be placed above that typedef.

Other than that, this looks good to me. The result is perhaps not as tidy as we would like (especially with some functions being documented and others not) but I think, anyway, that a verbatim movement should be done in one commit (this one) and improvements, in a subsequent commit.