On Sat, Oct 19, 2019 at 11:07 AM Jonathan Tan <jonathanta...@google.com> wrote:
>
> > From: Heba Waly <heba.w...@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
>
Ok.
> Also, include the deletion of api-config.txt in this commit.
I wasn't sure if the api-config.txt should be removed or not so I
decided to keep it
and wait for feedback. I assume I'll need to delete api-config.html as well?
> If you are doing any summarizing, describe what summarizing you are
> doing in the commit message too.
Ok, will do so.
> > + * 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.
>
Cool, I couldn't find it, thanks!
> 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.
You're right, I would have loved to get all the functions documented, but that's
something I'm not able to do right now as I'm still getting familiar
with the code base.
But it's a good start!
I agree with you about moving the documentation and deleting the file
in one commit.
Will do so.
Thank you for the feedback!
Heba
