Re: [PATCH v2] blame: add blame.showEmail configuration

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

 



Quentin Neill <quentin.neill@xxxxxxxxx> writes:

> From: Quentin Neill <quentin.neill@xxxxxxxxx>
>
> Complement existing --show-email option with fallback
> configuration variable, with tests.
> ---

The patch itself looks very reasonable.  Thanks for getting back to
us ;-)

A few minor nits:

    - Your in-body "From:" is redundant and unnecessary, as your
      e-mail is coming from the same address.

    - You need "Signed-off-by: Quentin Neill <quentin.neill@xxxxxxxxx>"
      after your log message (separate it with a blank line before
      the sign-off, and place the sign-off before the three-dash
      lines).

> diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
> index 5cdf3f1..faf1660 100755
> --- a/t/t8002-blame.sh
> +++ b/t/t8002-blame.sh
> @@ -19,4 +19,66 @@ test_expect_success 'blame --show-email' '
>  		"<E at test dot git>" 1
>  '
>  
> +test_expect_success 'setup showEmail tests' '
> +	echo "bin: test number 1" >one &&
> +	git add . &&
> +	GIT_AUTHOR_NAME=name1 GIT_AUTHOR_EMAIL=email1@xxxxxxxx git commit -a -m First --date="2010-01-01 01:00:00"
> +'
> +
> +cat >expected_n <<\EOF &&
> +(name1 2010-01-01 01:00:00 +0000 1) bin: test number 1
> +EOF
> +
> +cat >expected_e <<\EOF &&
> +(<email1@xxxxxxxx> 2010-01-01 01:00:00 +0000 1) bin: test number 1
> +EOF

These two commands outside test_expect_success are part of setup, so

	test_expect_success 'setup showEmail tests' '
        	echo "bin: test number 1" >one &&
                git add one &&
                GIT_AUTHOR_NAME=name1 \
                GIT_AUTHOR_EMAIL=email1@xxxxxxxx \
                git commit -m First --date="2010-01-01 01:00:00" &&
		cat >expected_n <<-\EOF &&
                (name1 ...
                EOF
                cat >expected_e <<-\EOF
                (<email1@...
		EOF
	'

Also do not hesitate to break overlong lines with "\".

> +find_blame() {

style: "find_blame () {"

Other than that, the patch looks good.

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