On Sun, May 31, 2015 at 1:13 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Quentin Neill <quentin.ne...@gmail.com> writes:
>
>> From: Quentin Neill <quentin.ne...@gmail.com>
>>
>> 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.
I tried using 'git send-email' directly on the commit, not sure how or
why that occurred.
I will fall back to 'git format-patch' and 'git send-email' as I did
in my first post.
> - You need "Signed-off-by: Quentin Neill <quentin.ne...@gmail.com>"
> 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=ema...@test.git 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 &&
>> +(<ema...@test.git> 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=ema...@test.git \
> 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.
Thanks for the help, one more version on the way.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
