Re: [PATCH] doc: fix the stale link to api-builtin.txt
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Apr 15, 2020, 03:14 UTC
- Message-ID
- <20200415031429.GA36683@google.com>
- In-Reply-To
- <pull.647.git.git.1586882575822.gitgitgadget@gmail.com>
(+Emily Shaffer, author of the related MyFirstContribution tutorial) Hi,
Kazuo Yagi wrote:
> From: Kazuo Yagi <kazuo.yagi@gmail.com> > Subject: doc: fix the stale link to api-builtin.txt > > ec14d4e had moved documentation from api-builtin.txt to builtin.h.
Micronits: - use "git log -1 --format=reference" - changes due to previous commits go in the simple past tense, like "Commit ec14d4e moved [...]"
> This patch updates new-command.txt to reflect that change.
nit: describe what your patch does as though you are giving orders to the codebase to change. Focus on what benefit you are trying to bring about.
> Signed-off-by: Kazuo Yagi <kazuo.yagi@gmail.com> > ---
[...]
> along with the changeset history.
This part didn't make it into the commit message, but I think you intended it to do so. Putting that all together would make
new-command doc: fix stale link to api-builtin.txt
ec14d4ecb55 (builtin.h: take over documentation from api-builtin.txt, 2017-08-02) moved the documentation for Git's builtin API to the header file. Update the "new command" how-to doc to reflect that change.
While we're here, add a historical note to builtin.h.
[...]
Show 7 quoted lines
> --- a/Documentation/howto/new-command.txt > +++ b/Documentation/howto/new-command.txt > @@ -1,13 +1,13 @@ > From: Eric S. Raymond <esr@thyrsus.com> > Abstract: This is how-to documentation for people who want to add extension > - commands to Git. It should be read alongside api-builtin.txt. > + commands to Git. It should be read alongside builtin.h.
Makes sense.
[...]
Show 6 quoted lines
> How to integrate new subcommands > ================================ > > This is how-to documentation for people who want to add extension > -commands to Git. It should be read alongside api-builtin.txt. > +commands to Git. It should be read alongside builtin.h.
Likewise. It's probably worth pointing to Documentation/MyFirstContribution as well, which is a tutorial covering this subject in more detail.
(Actually, would it make sense to incorporate the information from howto/new-command.txt into that page and then to retire the old new-command doc?)
[...]
Show 7 quoted lines
> @@ -48,7 +48,7 @@ binary); this organization makes it easy for people reading the code > to find things. > > See the CodingGuidelines document for other guidance on what we consider > -good practice in C and shell, and api-builtin.txt for the support > +good practice in C and shell, and builtin.h for the support > functions available to built-in commands written in C.
Most support functions are part of other APIs, so this was a strange pointer in the first place. But the change makes sense.
It's also a bit odd that this doc doesn't mention SubmittingPatches.
[...]
Show 12 quoted lines
> --- a/builtin.h > +++ b/builtin.h > @@ -92,6 +92,31 @@ > * > * The return value from `cmd_foo()` becomes the exit status of the > * command. > + * > + * Changeset History > + * ----------------- > + * > + * The following describes how the documentation has finally been placed > + * in this file, over the related changesets.
*puzzled* Why is this information being added to the builtin.h file? What is the reader trying to do when they read it?
Thanks and hope that helps, Jonathan
Show 22 quoted lines
> + * > + * +-----------------+ *OLD LINK* +-----------------+ > + * | api-builtin.txt | <~~~~~~~~~~ | api-command.txt | > + * +-----------------+ +-----------------+ > + * | ~ * | > + * | deleted, ~ N | moved and renamed from > + * | contents is taken over ~ E | Documentation/technical/ > + * | by builtin.h ~ W | to > + * | (this file) ~ | Documentation/howto/ > + * | ~ L | > + * | ~ I | > + * v ~ N v > + * +-----------+ ~ K +-----------------+ > + * | builtin.h | <~~~~~~~~~~~~ * | new-command.txt | > + * +-----------+ +-----------------+ > + * > + * ---> moved to(or renamed to) > + * ~~~> refers to > + * > */ > > #define DEFAULT_MERGE_LOG_LEN 20