Re: [PATCH] Start conforming code to "git subcmd" style

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Heikki Orsila <heikki.orsila@xxxxxx> writes:

> User notifications are presented as 'git cmd', and code comments
> are presented as '"cmd"' or 'git's cmd', rather than 'git-cmd'.

Thanks.

The part I will _not_ comment on in your patch looked all good, so to
reduce further back-and-forth, I'd apply them to 'maint', excluding the
parts I did comment on in this message.

> diff --git a/builtin-apply.c b/builtin-apply.c
> ...
> @@ -506,17 +506,17 @@ static char *gitdiff_verify_name(const char *li
> ...
> -			die("git-apply: bad git-diff - expected /dev/nu...
> +			die("git apply: bad git-diff - expected /dev/nu...
> ...
> -			die("git-apply: bad git-diff - inconsistent %s ...
> +			die("git apply: bad git-diff - inconsistent %s ...
> ...
> -			die("git-apply: bad git-diff - expected /dev/nu...
> +			die("git apply: bad git-diff - expected /dev/nu...
> ...

I'd vote for doing "s/git-diff/patch/" here.  After looking at
builtin-apply.c, there is no other error/die messages that would become
ambiguous, so such a rewording won't make it harder to help people who saw
any of these error messages (or other error messages from the "git-apply"
program).

> diff --git a/builtin-blame.c b/builtin-blame.c
> ...
> @@ -2299,12 +2299,12 @@ int cmd_blame(int argc, const char **argv, co...
> ...
> -		OPT_BIT(..."Use the ... as git-annotate (Default: off)"...
> +		OPT_BIT(..."Use the ... as git annotate (Default: off)"...
> ...
> -		OPT_STRING(..."Use ...instead of calling git-rev-list"),
> +		OPT_STRING(..."Use ...instead of calling git rev-list"),
> ...

A two-word command name in a prose is hard to read; "rev-list" is not a
word and that makes the problem less serious, but it would be easier to
read if these two word command names are quoted or grouped together in
some way to make it clear they form a single noun and the sentence is
talking about a single "thing".

The old "git-foo" spelling was good for that purpose, but it will invite
user confusion so we cannot use it anymore.  Perhaps we can say "instead
of calling 'git rev-list'"?

The command name at the beginning of die message does not have this issue.
E.g. the colon in:

	die("git foo: I hate you");

is sufficient to make it clear that these two words form a single noun;
i.e. "I'm 'git foo' program, and I am telling you that I hate you".

But it might be just me, so before asking you to reroll another round, I'd
like to hear opinions from the list.

 (1) No, JC is worrying too much about readability; Heikki's patch is good;

 (2) JC's right -- "instead of calling 'git rev-list'" is much better;

 (3) Something else?

> diff --git a/builtin-branch.c b/builtin-branch.c
> @@ -526,7 +526,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> ...
> -		OPT_GROUP("Specific git-branch actions:"),
> +		OPT_GROUP("Specific git branch actions:"),
> ...

Likewise.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux