Re: [PATCH v3] Documentation/git-config.txt: reword missleading sentence

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 18, 2017, 22:12 UTC
Message-ID: <20171018220530.h6dgneq2dvhmwfeu@aiede.mtv.corp.google.com>
In-Reply-To: <20171018202716.13313-1-nathan.payre@etu.univ-lyon1.fr>

Hi Nathan et al,

PAYRE NATHAN p1508475 wrote:

> From: PAYRE NATHAN p1508475 <second.payre@gmail.com>

nit: this 'From' line doesn't match any of the authors with sign-offs below. I'm wondering if the authorship of the commit (from "git commit --author" or git's "user.name" / "user.email" settings) made it through in the email.

If you pass --from='PAYRE NATHAN p1508475 <second.payre@gmail.com>' to "git format-patch", then it will send email from that address and add an in-body "From:" line to the message it produces based on the author field of the git commits. This can be useful for sending patches that someone else wrote. This way, the authorship of the commit gets through, to be used by "git am".

See https://public-inbox.org/git/20170922203017.53986-7-git@jeffhostetler.com/ for an example of how that works.

> Subject: Documentation/git-config.txt: reword missleading sentence

more nits:

- this subject line can use a more concise name for the subsystem,
  like "config doc"

- there's one too many esses in missleading

- more importantly, this is a good place to provide a summary of what
  change the patch makes, so people reading the shortlog in Git
  release announcements know what changed. Maybe something like
  "use a clearer placeholder in example"?

That would mean something like

	config doc: use a clearer placeholder in example

Or, to capture the other changes being made at the same time:

	config doc: clarify "git config --path" example

Thanks for working on this.

> Change the word "bla" to "section.variable", "bla" is a placeholder
> for a variable name and it wasn't clear for everyone.  This change
> clarify it.

The "," should be a semicolon or period, since it separates two independent clauses.

I think the "This change clarify it" sentence can go away. The first sentence already explains that this is for clarity.

> Change the appearance of 'git config section.variable {tilde}/' to
> `git config section.variable ~/` to harmonize it with the rest of the
> file, this is a command line then the "`" are necessary.

I had to read this sentence a few times to understand it. Maybe:

	While we're here, also reformat this sample command line to use
	monospace instead of italics, to better match the rest of the file.

> Replace "git-config" by "git config" because the command is not
> "git-config".

My first reaction to that is, "But the manpage is git-config(1)!" To avoid that reaction, it can say

	Use a space instead of a dash in "git config", as is common in the
	rest of Git's documentation.

By the way, should the initial `git config` be 'git config' (in italics instead of monospace)? I don't see `git config` anywhere else in the file and there are a lot of instances of 'git config'.

> See discussion at:
> https://public-inbox.org/git/20171002061303.Horde.SL92grZCqTRV9oQkBFPELQ7@crashcourse.ca/

The commit message is meant to be (and is, I think!) self-contained, so this link shouldn't be needed for people running into this change in "git log". I'd leave it out.

Show 5 quoted lines

> Signed-off-by: MOY Matthieu <matthieu.moy@univ-lyon1.fr>
> Signed-off-by: Daniel Bensoussan <daniel.bensoussan--bohm@etu.univ-lyon1.fr>
> Signed-off-by: Timothee Albertin <timothee.albertin@etu.univ-lyon1.fr>
> Signed-off-by: Nathan Payre <nathan.payre@etu.univ-lyon1.fr>
> Noticed-by: rpjday@crashcourse.ca

optional:

Reported-by: Robert P. J. Day <rpjday@crashcourse.ca>

This can go before the sign-offs to reflect the chronology.

[...]

Show 8 quoted lines

> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -174,11 +174,11 @@ See also <<FILES>>.
>  	either --bool or --int, as described above.
>  
>  --path::
> -	'git-config' will expand leading '{tilde}' to the value of
> +	`git config` will expand leading `~` to the value of

not about this patch: the article "a" is missing before "leading `~`":

	'git config' will expand a leading `~` to the value of ...

Show 7 quoted lines

> -	'$HOME', and '{tilde}user' to the home directory for the
> +	`$HOME`, and `~user` to the home directory for the
>  	specified user.  This option has no effect when setting the
> -	value (but you can use 'git config bla {tilde}/' from the
> -	command line to let your shell do the expansion).
> +	value (but you can use `git config section.variable ~/`
> +	from the command line to let your shell do the expansion).

I like it.

Thanks and hope that helps, Jonathan