From: Heba Waly <heba.waly@gmail.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: gitgitgadget@gmail.com, git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH 1/1] config: add documentation to config.h
Date: Sun, 20 Oct 2019 21:05:27 +1300 [thread overview]
Message-ID: <CACg5j25HsQwjjruHJ2E7F6UF4jP70x436km8r52qpzNSHXfpbA@mail.gmail.com> (raw)
In-Reply-To: <20191018220705.241778-1-jonathantanmy@google.com>
On Sat, Oct 19, 2019 at 11:07 AM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> > 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
>
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
next prev parent reply other threads:[~2019-10-20 8:05 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-18 0:06 [PATCH 0/1] config: add documentation to config.h Heba Waly via GitGitGadget
2019-10-18 0:06 ` [PATCH 1/1] " Heba Waly via GitGitGadget
2019-10-18 22:07 ` Jonathan Tan
2019-10-20 8:05 ` Heba Waly [this message]
2019-10-18 22:51 ` Emily Shaffer
2019-10-20 8:35 ` Heba Waly
2019-10-22 20:42 ` Emily Shaffer
2019-10-22 7:05 ` [PATCH v2 0/1] [Outreachy] config: move " Heba Waly via GitGitGadget
2019-10-22 7:05 ` [PATCH v2 1/1] " Heba Waly via GitGitGadget
2019-10-22 20:59 ` Emily Shaffer
2019-10-23 2:14 ` Junio C Hamano
2019-10-23 4:55 ` Heba Waly
2019-10-23 5:30 ` [PATCH v3 0/1] [Outreachy] " Heba Waly via GitGitGadget
2019-10-23 5:30 ` [PATCH v3 1/1] " Heba Waly via GitGitGadget
2019-10-23 21:38 ` Emily Shaffer
2019-10-24 2:14 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACg5j25HsQwjjruHJ2E7F6UF4jP70x436km8r52qpzNSHXfpbA@mail.gmail.com \
--to=heba.waly@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=jonathantanmy@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).