Re: [PATCH v2 15/20] parse-options: move doc to parse-options.h

From: Emily Shaffer <emilyshaffer@google.com>
Date: Nov 15, 2019, 23:28 UTC
Message-ID: <20191115232805.GF22855@google.com>
In-Reply-To: <xmqqv9rl5q0w.fsf@gitster-ct.c.googlers.com>

On Fri, Nov 15, 2019 at 08:37:35PM +0900, Junio C Hamano wrote:

Show 36 quoted lines

> Heba Waly <heba.waly@gmail.com> writes:
> 
> >> Note that the quality of the latter is quite uneven.  The one I
> >> noticed perhaps is exceptionally well-structured (even if some of
> >> the details of its contents may have gotten stale) ...
> > ...
> > Back to the main issue you raised in the beginning: lets look at two
> > different scenarios before and after moving the docs, the first
> > scenario is the one you've been through trying to look for
> > PARSE_OPT_STOP_AT_NON_OPTION, luckily you found it in
> > Documentation/technical/api-parse-options.txt and the doc was pretty
> > useful for you. But if you were looking for PARSE_OPT_ONE_SHOT
> > instead, which is not documented in the doc file, you'd have ended up
> > in parse-options.h with no documentation, even searching for the enum
> > parse_opt_flags itself won't lead to anything useful I assume.
> 
> That is a longwinded way to say that the Doc version is outdated.  I
> am *not* advocating to keep it up-to-date just like the header (I've
> given up on that), but if it were kept fresh, then those who looked
> for ONE_SHOT would have the same ease to learn where in the larger
> picture it fits, like I did with STOP_AT_NON_OPTION.
> 
> It is not like "In the header file, it is likely that we can keep
> these up to date more easily.  A dedicated file in Documentation/
> hiearchy may be able to offer you a much better structure, but may
> not describe the option at all.  Which one do you want?"  At least,
> it should not have to be.  That is what I meant when I responded to
> your earlier
> 
> > So my proposal for this matter is to investigate the possibility of
> > using a doc generators that'd extract the documentations from the code
> > to a single doc file per library.
> 
> Extracting is just the necessary first step.  It would probably need
> to let us leave notes (in the header file used as the source of the
> documentation) to reorder things for ease of reading through.

I think doc generation from headers is a reasonable goal, although a much larger one than "get rid of the worst offenders in Documentation/technical".

Show 11 quoted lines

> 
> > last version of this patch, if we're trying to look for either
> > PARSE_OPT_STOP_AT_NON_OPTION or PARSE_OPT_ONE_SHOT, we'll find that
> > it's a member of parse_opt_flags, searching for the enum will lead us
> > to ....
> 
> ... which is what I found to be a frustrating experience of being
> forced to jump around to hunt for the necessary pieces of info
> sprinkled in the header file to form the overall picture, that a
> simple flat-file text document would have much easily given me
> (i.e. go back to my original post).

For what it's worth, I tend to agree that api-parse-options.txt is a particularly well-structured documentation page. I like that one and refer to it often.

Having read this back and forth, my own take is something like this:

 - Like Junio said, the quality of the the contents of
   Documentation/technical/ ranges from useless to excellent.
 - It's true that moving documentation into a header denies us some of
   the nice organization of a freeform doc.
 - The nice thing about headers is that we can reorganize declarations
   however we want, and if doing so makes the code more readable, then
   I think we should.
 - But, if we won't get up to the same level of readability as the old
   doc, then I don't see a reason to "throw the baby out with the
   bathwater," so to speak. It seems like api-parse-options.txt is good
   enough to stay.

By the way, this does make me start to wonder about the feasibility of linking in documentation generated from code comments, or other sorts of trickery to get the best of both worlds. But again, that's a very different task from "get rid of misleading or useless docs". :)

For now, maybe it's enough to add a link to api-parse-options.txt at the top of parse-options.h. Part of me wants to say maybe we should duplicate the per-function briefs into the header too, but then we have two sources, and a single one is easier to keep fresh.

 - Emily