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.

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

Reply via email to