From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sat, 04 Feb 2012 20:10:27 GMT
Subject: Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
Message-ID: <20120204201027.GE22928@burratino>
In-Reply-To: <1328385024-6955-3-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras wrote:

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

Thanks for writing tests.  I think there is room for a few lines of
explanation above.

[...]
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -255,4 +255,22 @@ test_expect_success 'Blame output (complex mapping)' '
>  	test_cmp expect actual.fuzz
>  '
>  
> +# git blame
> +cat >expect <<\EOF
> +^OBJI (<author@example.com>       DATE 1) one
> +OBJID (<some@dude.xx>             DATE 2) two
> +OBJID (<other@author.xx>          DATE 3) three
> +OBJID (<other@author.xx>          DATE 4) four
> +OBJID (<santa.claus@northpole.xx> DATE 5) five
> +OBJID (<santa.claus@northpole.xx> DATE 6) six
> +OBJID (<cto@company.xx>           DATE 7) seven
> +EOF
> +test_expect_success 'Blame output (complex mapping)' '

Since I didn't receive a copy of the cover letter or patch 1, I don't
know what this is intended to test _for_.  Good --- I can more easily
convey the reaction of future readers who do not necessarily know the
context in which the patch was written (and the commit message does
not seem to say).

Looking above, I see

 - a lone comment "git blame".  What is it trying to tell me?  I guess
   you copy/pasted it, but is there any purpose to it?

 - a test asserting the claim "Blame output (complex mapping)".  This
   title is identical to the test before.  I have no idea what this is
   about.

Puzzled,
Jonathan