[PATCH] doc: fix the stale link to api-builtin.txt

[PATCH] doc: fix the stale link to api-builtin.txt

[Kazuo Yagi via GitGitGadget]

From: Kazuo Yagi <kazuo.yagi@gmail.com>

ec14d4e had moved documentation from api-builtin.txt to builtin.h.
This patch updates new-command.txt to reflect that change.

Signed-off-by: Kazuo Yagi <kazuo.yagi@gmail.com>
---
    Fixed unavailable link in Documentation/howto/new-command.txt along…
    
    … with the changeset history.
    
    Signed-off-by: Kazuo Yagi kazuo.yagi@gmail.com [kazuo.yagi@gmail.com]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-647%2Fkyagi%2Ffix-unavailable-link-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-647/kyagi/fix-unavailable-link-v1
Pull-Request: https://github.com/git/git/pull/647

 Documentation/howto/new-command.txt |  6 +++---
 builtin.h                           | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/Documentation/howto/new-command.txt b/Documentation/howto/new-command.txt
index 15a4c8031f1..ac73c98be72 100644
--- 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.
 Content-type: text/asciidoc
 
 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.
 
 Runtime environment
 -------------------
@@ -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.
 
 What every extension command needs
diff --git a/builtin.h b/builtin.h
index 5cf5df69f72..101ef8edc4d 100644
--- 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.
+ *
+ * +-----------------+ *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

base-commit: 3cb8921f74354a3a4aeaa932869acb7e6aabe630
-- 
gitgitgadget

Re: [PATCH] doc: fix the stale link to api-builtin.txt

[Jonathan Nieder]

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

[...]
> --- 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.

[...]
>  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?)

[...]
> @@ -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.

[...]
> --- 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

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

Re: [PATCH] doc: fix the stale link to api-builtin.txt

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

>> + * 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 for an excellent review, but you are being a bit too subtle
and/or diplomatic here.

A good rule of thumb to use when judging if a comment is appropriate
to have in the tracked data is if it talks about what used to be the
case in order to explain why it is in the current shape.  Often,
such description is useless for people going forward starting from
the current codebase, and is better described in the log message,
and I think that this is a prime example.  As an explanation to
justify why it is good to refer to builtin.h from the current
documentation that teaches what needs to be done to add a new
command, instead of api-builtin.txt, it is valuable to know how the
description of the API used to support builtin commands moved over
time from place to place (preferrably with references to the commits
that did so), and it belongs to the log message of this commit that
updates the reference to api-builtin.txt to builtin.h.