Subject: Re: [PATCH v3 1/4] doc: move author and committer information to git-commit(1)

From: Jakub Narebski <[email-masked]>
Date: Sun, 03 Nov 2019 12:13:51 +0100

"brian m. carlson" <[email-masked]> writes:

> While at one time it made perfect sense to store information about
> configuring author and committer information in the documentation for
> git commit-tree, in modern Git that operation is seldom used.  Most
> users will use git commit and expect to find comprehensive documentation
> about its use in the manual page for that command.
>
> Considering that there is significant confusion about how one is to use
> the user.name and user.email variables, let's put as much documentation
> as possible into an obvious place where users will be more likely to
> find it.
>
> Signed-off-by: brian m. carlson <[email-masked]>
> ---
>  Documentation/config/user.txt     |  2 +-
>  Documentation/git-commit-tree.txt | 39 ++++---------------------------
>  Documentation/git-commit.txt      | 34 ++++++++++++++++++++++++---
>  3 files changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt
> index 0557cbbceb..a1f80e823c 100644
> --- a/Documentation/config/user.txt
> +++ b/Documentation/config/user.txt
> @@ -13,7 +13,7 @@ committer.email::
>  	Also, all of these can be overridden by the `GIT_AUTHOR_NAME`,
>  	`GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_NAME`,
>  	`GIT_COMMITTER_EMAIL` and `EMAIL` environment variables.
> -	See linkgit:git-commit-tree[1] for more information.
> +	See linkgit:git-commit[1] for more information.

O.K., lets refer users to the command they actually use.

>  user.useConfigOnly::
>  	Instruct Git to avoid trying to guess defaults for `user.email`
> diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
> index 4b90b9c12a..d518dffce0 100644
> --- a/Documentation/git-commit-tree.txt
> +++ b/Documentation/git-commit-tree.txt
> @@ -42,6 +42,10 @@ tend to just write the result to the file that is pointed at by
>  `.git/HEAD`, so that we can always see what the last committed
>  state was.
>  
> +A commit comment is read from stdin. If a changelog
> +entry is not provided via "<" redirection, 'git commit-tree' will just wait
> +for one to be entered and terminated with ^D.
> +

I understand this is what is extracted from the removed part (moved to
git-commit documentation).  But I think it is not enough to understand
how to actually use this command; this change makes reference
documentation not self contained.

>  OPTIONS
>  -------
>  <tree>::
> @@ -69,40 +73,6 @@ OPTIONS
>  	Do not GPG-sign commit, to countermand a `--gpg-sign` option
>  	given earlier on the command line.
>  
> -
> -Commit Information
> -------------------
> -
> -A commit encapsulates:
> -
> -- all parent object ids
> -- author name, email and date
> -- committer name and email and the commit time.

O.K., I can understand moving this information to a better place
(nb. what about gitglossry?).

> -
> -While parent object ids are provided on the command line, author and
> -committer information is taken from the following environment variables,
> -if set:
> -
> -	GIT_AUTHOR_NAME
> -	GIT_AUTHOR_EMAIL
> -	GIT_AUTHOR_DATE
> -	GIT_COMMITTER_NAME
> -	GIT_COMMITTER_EMAIL
> -	GIT_COMMITTER_DATE
> -
> -(nb "<", ">" and "\n"s are stripped)
> -
> -In case (some of) these environment variables are not set, the information
> -is taken from the configuration items user.name and user.email, or, if not
> -present, the environment variable EMAIL, or, if that is not set,
> -system user name and the hostname used for outgoing mail (taken
> -from `/etc/mailname` and falling back to the fully qualified hostname when
> -that file does not exist).

I'm not sure if this should not be left here, perhaps extracted into
separate fragment file to be included in both git-commit-tree.txt and in
git-commit.txt

I see that we are missing the ebvious information that if GIT_*_DATE
environment variables are not set, Git will use current date and time.
Is there a place where we say what format GIT_*_DATE needs to be in?

> -
> -A commit comment is read from stdin. If a changelog
> -entry is not provided via "<" redirection, 'git commit-tree' will just wait
> -for one to be entered and terminated with ^D.
> -
>  include::date-formats.txt[]
>  
>  Discussion
> @@ -117,6 +87,7 @@ FILES
>  SEE ALSO
>  --------
>  linkgit:git-write-tree[1]
> +linkgit:git-commit[1]

All right.

>  
>  GIT
>  ---
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index afa7b75a23..f684f7fdc2 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -352,9 +352,6 @@ changes to tracked files.
>  	these files are also staged for the next commit on top
>  	of what have been staged before.
>  
> -:git-commit: 1
> -include::date-formats.txt[]
> -

All right, it is moved down, not removed.

>  EXAMPLES
>  --------
>  When recording your own work, the contents of modified files in
> @@ -448,6 +445,37 @@ alter the order the changes are committed, because the merge
>  should be recorded as a single commit.  In fact, the command
>  refuses to run when given pathnames (but see `-i` option).
>  
> +COMMIT INFORMATION
> +------------------
> +
> +A commit encapsulates:
> +
> +- all parent object ids
> +- author name, email and date
> +- committer name and email and the commit time.
> +
> +While parent object ids are provided on the command line,
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This was true for `git commit-tree`, with the following synopsis

  git commit-tree <tree> [(-p <parent>)…​]
  git commit-tree [(-p <parent>)…​] [-S[<keyid>]] [(-m <message>)…​]
  		  [(-F <file>)…​] <tree>

This is decidely *NOT true* for `git commit`:

  git commit [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]
  	   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]
  	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
  	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
  	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
  	   [-i | -o] [-S[<keyid>]] [--] [<file>…​]

Parent ids are *not* provided on the command line.

>                                                            author and
> +committer information is taken from the following environment variables,
> +if set:
> +
> +	GIT_AUTHOR_NAME
> +	GIT_AUTHOR_EMAIL
> +	GIT_AUTHOR_DATE
> +	GIT_COMMITTER_NAME
> +	GIT_COMMITTER_EMAIL
> +	GIT_COMMITTER_DATE
> +
> +(nb "<", ">" and "\n"s are stripped)
> +
> +In case (some of) these environment variables are not set, the information
> +is taken from the configuration items user.name and user.email, or, if not
> +present, the environment variable EMAIL, or, if that is not set,
> +system user name and the hostname used for outgoing mail (taken
> +from `/etc/mailname` and falling back to the fully qualified hostname when
> +that file does not exist).

I think that this description, while perfectly fine for plumbing command
`git commit-tree`, might be needlesly confusing for `git commit` users,
as it describes less often common used way of providing committer and
author information first.

Also in `git commit` there are *options* specifically for adjusting how
committer and author information is handled: `--reuse-message=<commit>`
and `--reedit-message=<commit>`, `--reset-author` (doesn't make sense
alone), `--author=<author>` and `--date=<date>`.

Maybe describe usual way first, then describe the various options and
their priority?

> +
> +:git-commit: 1
> +include::date-formats.txt[]
>  
>  DISCUSSION
>  ----------

Best,
-- 
Jakub Narębski