Re: [PATCH v2 1/3] send-email: print newline for --git-completion-helper

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

 



Thiago Perrotta <tbperrotta@xxxxxxxxx> writes:

> Rationale: Currently all git built-in commands print a newline upon upon git
> <cmd> --git-completion-helper. Therefore git-send-email should follow suit for
> consistency.

"upon upon".

You do not need to start with "Rationale", because one of the things
you need to have in any proposed log message is a justification for
the change.  You also do not need "Currently" in our log message, as
the convention here is to state the status quo in the present tense,
point out what's wrong there (or leave it unsaid and implied by the
description of the current state, if it is obvious), state the
approach to correct what's wrong, and finally give an order to the
codebase to "become like so".

	Unlike other Git subcommands, "git send-email" leaves its
	output an incomplete line when "--git-completion-helper" is
	asked.  Be consistent by terminating the message with LF
	here.

or something like that.

I do not know which style is preferred among

 (1)	print something;
	print "\n";

 (2)	print something, "\n";

 (3)	print something . "\n";

but other than that, the goal and the implementation both sound
sensible (the second one is what I'd be writing if I were doing this
change myself, FWIW).

Thanks.

> Signed-off-by: Thiago Perrotta <tbperrotta@xxxxxxxxx>
> ---
>  git-send-email.perl | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index e65d969d0b..e991bf333d 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -115,6 +115,7 @@ sub usage {
>  
>  sub completion_helper {
>      print Git::command('format-patch', '--git-completion-helper');
> +    print "\n";
>      exit(0);
>  }



[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