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

[Quentin Neill <quentin.neill@gmail.com>]

On Sun, May 31, 2015 at 1:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Quentin Neill <quentin.neill@gmail.com> writes:
>
>> From: Quentin Neill <quentin.neill@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.neill@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=email1@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 &&
>> +(<email1@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=email1@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.