git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Stefan Beller <sbeller@google.com>,
	Junio C Hamano <gitster@pobox.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [PATCH] document string_list_clear
Date: Fri, 12 Dec 2014 14:24:17 -0500	[thread overview]
Message-ID: <20141212192417.GA21132@peff.net> (raw)
In-Reply-To: <20141212183114.GA29365@google.com>

On Fri, Dec 12, 2014 at 10:31:14AM -0800, Jonathan Nieder wrote:

> Separate from the question of history, I honestly prefer this way of
> doing API documentation relative to 90% of the API documentation in
> headers I've seen in other projects.  I suspect you don't.  That's
> okay --- it's possible for rational people to disagree about things.
> 
> It's moot given that we don't seem to disagree about what should be
> done about it.  Why keep arguing?

Fair enough.

> > I think the end result that I posted is still strictly better than what
> > we have currently, with the exception that I should have reformatted the
> > hanging indents. What is it that you don't like about it?
> 
> Other issues of inconsistent markup.  For example, some comments are
> strangely indented, and some look like
> 
> 	/* First line
> 	 * second line
> 	 * third line */

Yeah, I had to strip the indent from:

  strbuf_init::
	description of strbuf_init...

when "strbuf_init::" went away, but it looks like I missed two of them.
And I see one "/* First line" where the "header" from "Adding data to
the buffer" got merged with a note. I agree those should be fixed.

I used our normal single-line comment style for most of those:

  /* Related to the size of the buffer */

in some places. But I'd be happy, too, with:

  /*
   * Related to the size of the buffer
   */

(and probably using a full sentence, or caps or underlining or something
to indicate that the comment begins a new section of the header file).

> I chose not to list markup issues for the same reason (it's more
> tedious to go back and forth than for someone to spend some time to
> get it mostly-right first on their own).  I am kind of confused about
> the current status, since I've said again and again that I'd be happy
> to see the documentation in the header but you still seem to be trying
> to convince me of that.  What am I missing?

I think partially it was crossing mails. While I was writing the
response that you responded to just now, I ended up postponing it for
other work, and in the meantime you sent several more messages
indicating you were OK with moving documentation into the headers. I
almost scrapped my response, but frankly I was left a bit confused by
your position, since it seemed the opposite of the 2 patches you had
sent moving things out of headers.

Do not feel obligated to make me unconfused. If you are on board with
putting documentation in the headers, then I think we can move forward
with that plan.

So what next?

I agree there are some formatting problems in the strbuf.h patch I sent
earlier. I'm happy to fix them and resend, but I'm not 100% sure that
fixing all the problems I see will not leave problems for you. I can fix
them and you can review if you want. Or alternatively, if you have more
drastic formatting or wording changes in mind, maybe it would make sense
for you to take a pass?

I _don't_ want to commit to moving all of api-* into headers myself.
It's something I would rather have happen over time as people touch
areas[1], because it's tedious and time-consuming. That does create an
inconsistency, in the sense that some APIs will be documented in
Documentation/technical, and others in their header files. But as long
as each _individual_ API is documented in one or the other, I don't see
that as a big problem (of course the status quo is a mix for some APIs,
so we will live with that until they are touched, but that is no worse
than today's state).

-Peff

[1] I had a vague plan to put new documentation into headers, and slowly
    migrate that way until we had a critical mass. But that was moving
    somewhat slowly, and obviously escaped the notice of many people. :)

  parent reply	other threads:[~2014-12-12 19:24 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-06  1:51 [PATCH] document string_list_clear Stefan Beller
2014-12-06  2:04 ` Jonathan Nieder
2014-12-06  5:27   ` Jeff King
2014-12-06  5:30   ` Jeff King
2014-12-09 19:41     ` Junio C Hamano
2014-12-09 20:15       ` Jeff King
2014-12-09 20:21         ` Jonathan Nieder
2014-12-09 19:37   ` Junio C Hamano
2014-12-09 19:48     ` Stefan Beller
2014-12-09 20:17       ` Jonathan Nieder
2014-12-09 20:27         ` Jeff King
2014-12-09 20:32           ` Stefan Beller
2014-12-09 20:46             ` Jeff King
2014-12-09 22:23           ` Jonathan Nieder
2014-12-09 23:18             ` Junio C Hamano
2014-12-10  8:52               ` Jeff King
2014-12-10  8:43             ` Jeff King
2014-12-10  9:18               ` Jonathan Nieder
2014-12-12  9:16                 ` Jeff King
2014-12-12 18:31                   ` Jonathan Nieder
2014-12-12 19:17                     ` Junio C Hamano
2014-12-12 19:19                     ` Stefan Beller
2014-12-12 19:29                       ` Jeff King
2014-12-12 19:24                     ` Jeff King [this message]
2014-12-12 19:35                       ` Jonathan Nieder
2014-12-12 21:27                         ` Jeff King
2014-12-12 21:28                           ` [PATCH 1/4] strbuf: migrate api-strbuf.txt documentation to strbuf.h Jeff King
2014-12-12 21:40                             ` Jeff King
2014-12-12 22:16                             ` Junio C Hamano
2014-12-12 22:30                             ` Jonathan Nieder
2014-12-12 21:28                           ` [PATCH 2/4] strbuf.h: drop asciidoc list formatting from API docs Jeff King
2014-12-12 22:19                             ` Junio C Hamano
2014-12-12 22:37                             ` Jonathan Nieder
2014-12-12 21:30                           ` [PATCH 3/4] strbuf.h: format asciidoc code blocks as 4-space indent Jeff King
2014-12-12 22:39                             ` Jonathan Nieder
2014-12-14 17:42                               ` Michael Haggerty
2014-12-12 21:32                           ` [PATCH 4/4] strbuf.h: reorganize api function grouping headers Jeff King
2014-12-12 22:46                             ` Jonathan Nieder
2014-12-12 22:32                           ` [PATCH] document string_list_clear Stefan Beller
2014-12-10 20:09               ` Michael Haggerty
2014-12-10 21:51                 ` Jonathan Nieder
2014-12-10 22:28                   ` Junio C Hamano
2014-12-10 22:37                     ` Jonathan Nieder
2014-12-10 23:04                       ` Junio C Hamano
2014-12-10 23:08                         ` Jonathan Nieder
2014-12-09 22:49         ` Jonathan Nieder
2014-12-09 23:07           ` Stefan Beller
2014-12-09 23:15             ` Jonathan Nieder
2014-12-09 20:00     ` Jonathan Nieder

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=20141212192417.GA21132@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=sbeller@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.
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).