git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: phillip.wood@dunelm.org.uk
Cc: Stefan Xenos via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Christophe Poucet <christophe.poucet@gmail.com>
Subject: Re: [PATCH 05/10] evolve: add the change-table structure
Date: Wed, 28 Sep 2022 17:14:14 +0200	[thread overview]
Message-ID: <220928.86pmffwmft.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <72c4e8c9-62bb-96aa-7b42-d7103b7873eb@dunelm.org.uk>


On Wed, Sep 28 2022, Phillip Wood wrote:

> On 27/09/2022 16:28, Ævar Arnfjörð Bjarmason wrote:
>> On Tue, Sep 27 2022, Phillip Wood wrote:
>> 
>>> On 27/09/2022 14:50, Ævar Arnfjörð Bjarmason wrote:
>>>> On Tue, Sep 27 2022, Phillip Wood wrote:
>>>>
>>>>> On 23/09/2022 19:55, Stefan Xenos via GitGitGadget wrote:
>>>>>> +/**
>>>>>
>>>>> We tend to just use '/*' rather than '/**'
>>>> No, we use both, and /** is correct here. It's an API-doc syntax,
>>>> see
>>>> e.g. strbuf.h.
>>>> It's explicitly meant for this sort of thing, i.e. comments on
>>>> public
>>>> structs in a header & the functions in a header (and struct members,
>>>> etc.).
>>>
>>> We don't do that consistently, we don't mention them in
>>> CodingGuidelines and we don't use anything that processes API-doc
>>> comments. It would be a lot simpler and it would be consistent with
>>> our coding guidelines just to use the same style everywhere. That
>>> would avoid problems where this series uses API-doc comments for
>>> in-code comments in .c files and where single line comments in header
>>> files do not use the API-doc syntax.
>> Yes, this isn't documented in CodingGuidelines (but FWIW is in
>> various
>> commit messages).
>> I'm pointing out that this isn't a mistake, but the preferred style
>> for
>> new API docs.
>
> It seems a bit a stretch to call it the preferred style, however I had
> thought all our uses were historic but that's not the case.

FWIW that claim of mine comes from my (admittedly fuzzy recollection of)
history, around the time that this was being worked out (e.g. where API
docs should live, they used to mostly be in Documentation/technical/,
now they're mostly in *.h) we ended up with this mention in CodingGudielines:

 - When you come up with an API, document its functions and structures
   in the header file that exposes the API to its callers. Use what is
   in "strbuf.h" as a model for the appropriate tone and level of
   detail.

I.e. at some point we decreed strbuf.h as a best-practice model to
follow. I think every one of my own use of "/**" has come after reading
that file...

But patches to make it more explicit are most welcome. FWIW I think I
looked into that once, but couldn't find a canonical reference for what
this syntax is even called.

Hrm, looking a bit more I see it's probably JavaDoc (at least Emacs
highlights it as such). A grep of:

	git grep '^ \* @' -- '*.[ch]'

Shows that we make almost no use of the meta-syntax (i.e. "tags":
https://en.wikipedia.org/wiki/Javadoc#Table_of_Javadoc_tags)

> Chris if you want to use '/**' style comments for the API docs then
> please do so consistently and do not use them elsewhere.
>
>> At least Emacs knows how to highlight these differently, which is the
>> main use I personally get out of them, I don't know what other use-cases
>> there are for them...
>
> I've come across them in projects that use gtk-doc or other
> documentation generators where it is necessary to distinguish 
> 'documentation' from 'code comments'. I don't think they add much
> value if one is not generating documentation from the source, it is
> just one more thing for contributors to remember.

I for one find it very useful these "API docs" are shown differently in
my editor, even if I'm not using some fancier (and hypothetical) "make
javadoc" target.

I.e. you can ideally skim the *.h file and see at a glance what's an
implementation comment v.s. API doc comment.

Except that even for the supposed role model of strbuf.h we forgot in
some cases, and should have the below applied to it, oh well... :)

diff --git a/strbuf.h b/strbuf.h
index 76965a17d44..46549986ae3 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -492,7 +492,7 @@ int strbuf_getline_lf(struct strbuf *sb, FILE *fp);
 /* Uses NUL as the line terminator */
 int strbuf_getline_nul(struct strbuf *sb, FILE *fp);
 
-/*
+/**
  * Similar to strbuf_getline_lf(), but additionally treats a CR that
  * comes immediately before the LF as part of the terminator.
  * This is the most friendly version to be used to read "text" files
@@ -610,7 +610,7 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,
 	return strbuf_split_max(sb, terminator, 0);
 }
 
-/*
+/**
  * Adds all strings of a string list to the strbuf, separated by the given
  * separator.  For example, if sep is
  *   ', '
@@ -653,7 +653,7 @@ int launch_editor(const char *path, struct strbuf *buffer,
 int launch_sequence_editor(const char *path, struct strbuf *buffer,
 			   const char *const *env);
 
-/*
+/**
  * In contrast to `launch_editor()`, this function writes out the contents
  * of the specified file first, then clears the `buffer`, then launches
  * the editor and reads back in the file contents into the `buffer`.
@@ -693,7 +693,7 @@ static inline void strbuf_complete_line(struct strbuf *sb)
 	strbuf_complete(sb, '\n');
 }
 
-/*
+/**
  * Copy "name" to "sb", expanding any special @-marks as handled by
  * interpret_branch_name(). The result is a non-qualified branch name
  * (so "foo" or "origin/master" instead of "refs/heads/foo" or
@@ -707,7 +707,7 @@ static inline void strbuf_complete_line(struct strbuf *sb)
 void strbuf_branchname(struct strbuf *sb, const char *name,
 		       unsigned allowed);
 
-/*
+/**
  * Like strbuf_branchname() above, but confirm that the result is
  * syntactically valid to be used as a local branch name in refs/heads/.
  *

  reply	other threads:[~2022-09-28 15:26 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-23 18:55 [PATCH 00/10] Add the Git Change command Christophe Poucet via GitGitGadget
2022-09-23 18:55 ` [PATCH 01/10] technical doc: add a design doc for the evolve command Stefan Xenos via GitGitGadget
2022-09-23 19:59   ` Jerry Zhang
2022-09-28 21:26   ` Junio C Hamano
2022-09-28 22:20   ` Junio C Hamano
2022-09-29  9:17     ` Phillip Wood
2022-09-29 19:57   ` Jonathan Tan
2022-09-23 18:55 ` [PATCH 02/10] sha1-array: implement oid_array_readonly_contains Chris Poucet via GitGitGadget
2022-09-26 13:08   ` Phillip Wood
2022-09-23 18:55 ` [PATCH 03/10] ref-filter: add the metas namespace to ref-filter Chris Poucet via GitGitGadget
2022-09-26 13:13   ` Phillip Wood
2022-10-04  9:50     ` Chris P
2022-09-23 18:55 ` [PATCH 04/10] evolve: add support for parsing metacommits Stefan Xenos via GitGitGadget
2022-09-26 13:27   ` Phillip Wood
2022-10-04 11:21     ` Chris P
2022-10-04 14:10       ` Phillip Wood
2022-09-23 18:55 ` [PATCH 05/10] evolve: add the change-table structure Stefan Xenos via GitGitGadget
2022-09-27 13:27   ` Phillip Wood
2022-09-27 13:50     ` Ævar Arnfjörð Bjarmason
2022-09-27 14:13       ` Phillip Wood
2022-09-27 15:28         ` Ævar Arnfjörð Bjarmason
2022-09-28 14:33           ` Phillip Wood
2022-09-28 15:14             ` Ævar Arnfjörð Bjarmason [this message]
2022-09-28 15:59             ` Junio C Hamano
2022-09-27 14:18     ` Phillip Wood
2022-10-04 14:48     ` Chris P
2022-09-23 18:55 ` [PATCH 06/10] evolve: add support for writing metacommits Stefan Xenos via GitGitGadget
2022-09-28 14:27   ` Phillip Wood
2022-10-05  9:40     ` Chris P
2022-10-05 11:09       ` Phillip Wood
2022-09-23 18:55 ` [PATCH 07/10] evolve: implement the git change command Stefan Xenos via GitGitGadget
2022-09-25  9:10   ` Phillip Wood
2022-09-26  8:23     ` Ævar Arnfjörð Bjarmason
2022-09-26  8:25   ` Ævar Arnfjörð Bjarmason
2022-10-05 12:30     ` Chris P
2022-09-23 18:55 ` [PATCH 08/10] evolve: add the git change list command Stefan Xenos via GitGitGadget
2022-09-23 18:55 ` [PATCH 09/10] evolve: add delete command Chris Poucet via GitGitGadget
2022-09-26  8:38   ` Ævar Arnfjörð Bjarmason
2022-09-26  9:10     ` Chris Poucet
2022-09-23 18:55 ` [PATCH 10/10] evolve: add documentation for `git change` Chris Poucet via GitGitGadget
2022-09-25  8:41   ` Phillip Wood
2022-09-25  8:39 ` [PATCH 00/10] Add the Git Change command Phillip Wood
2022-10-04  9:33   ` Chris P
2022-10-04 14:24 ` Phillip Wood
2022-10-04 15:19   ` Chris P
2022-10-04 15:55     ` Chris P
2022-10-04 16:00       ` Phillip Wood
2022-10-04 15:57     ` Phillip Wood
2022-10-05 14:59 ` [PATCH v2 00/10] RFC: Git Evolve / Change Christophe Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 01/10] technical doc: add a design doc for the evolve command Stefan Xenos via GitGitGadget
2022-10-05 15:16     ` Chris Poucet
2022-10-06 20:53       ` Glen Choo
2022-10-10 19:35     ` Victoria Dye
2022-10-11  8:59       ` Phillip Wood
2022-10-11 16:59         ` Victoria Dye
2022-10-12 19:19           ` Phillip Wood
2022-10-05 14:59   ` [PATCH v2 02/10] sha1-array: implement oid_array_readonly_contains Chris Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 03/10] ref-filter: add the metas namespace to ref-filter Chris Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 04/10] evolve: add support for parsing metacommits Stefan Xenos via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 05/10] evolve: add the change-table structure Stefan Xenos via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 06/10] evolve: add support for writing metacommits Stefan Xenos via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 07/10] evolve: implement the git change command Stefan Xenos via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 08/10] evolve: add delete command Chris Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 09/10] evolve: add documentation for `git change` Chris Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 10/10] evolve: add tests for the git-change command Chris Poucet via GitGitGadget
2022-10-10  9:23   ` [PATCH v2 00/10] RFC: Git Evolve / Change Phillip Wood

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=220928.86pmffwmft.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=christophe.poucet@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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).