git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: gitgitgadget@gmail.com
Cc: git@vger.kernel.org, heba.waly@gmail.com, gitster@pobox.com,
	Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH 1/1] config: add documentation to config.h
Date: Fri, 18 Oct 2019 15:07:05 -0700	[thread overview]
Message-ID: <20191018220705.241778-1-jonathantanmy@google.com> (raw)
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.

> + * 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.

  reply	other threads:[~2019-10-18 22:18 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-18  0:06 [PATCH 0/1] " Heba Waly via GitGitGadget
2019-10-18  0:06 ` [PATCH 1/1] " Heba Waly via GitGitGadget
2019-10-18 22:07   ` Jonathan Tan [this message]
2019-10-20  8:05     ` Heba Waly
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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191018220705.241778-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=heba.waly@gmail.com \
    --subject='Re: [PATCH 1/1] config: add documentation to config.h' \
    /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

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 NNTP newsgroup(s).