From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Tue, 14 Feb 2012 21:35:33 GMT
Subject: Re: [PATCH 1/2] test: mailmap can change author name without changing email
Message-ID: <CAMP44s3di25SbMa1T1=0_s6f-rKnniwEcA+o5HWT7xedcghSeg@mail.gmail.com>
In-Reply-To: <20120214203519.GC13210@burratino>

On Tue, Feb 14, 2012 at 10:35 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> From: Felipe Contreras <felipe.contreras@gmail.com>

> test: mailmap can change author name without changing email

That doesn't say anything to me, which is weird if I supposedly wrote
this patch. What is the *purpose* of this?

At least 'add simple name translation test' is clear about the
purpose, albeit not clear enough as Junio pointed out.

> (2) 'email@example.com'
> becomes the canonical author email for commits with author name 'A U
> Thor'.

That's not true. I initially thought that was the case, and I think it
might be useful to have that, but it's not the case now, and your
patch doesn't test this.

> We already have tests for the effect (1) in the committer name, but
> not in the author name, so the tests do not cover the shortlog and
> blame codepaths as they should.  Fix that.

In order to test that you would need additional changes, something
along the lines of:

--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -157,8 +157,9 @@ A U Thor <author@example.com> (1):
 CTO <cto@company.xx> (1):
       seventh

-Committed <committer@example.com> (1):
+Committed <committer@example.com> (2):
       eighth
+      nine

 Other Author <other@author.xx> (2):
       third
@@ -204,6 +205,11 @@ test_expect_success 'Shortlog output (complex mapping)' '
        test_tick &&
        git commit --author "C O Mitter <committer@example.com>" -m eighth &&

+       echo nine >>one &&
+       git add one &&
+       test_tick &&
+       git commit --author "Committed <bad@example.com>" -m nine &&
+
        mkdir -p internal_mailmap &&
        echo "Committed <committer@example.com>" > internal_mailmap/.mailmap &&
        echo "<cto@company.xx>
<cto@coompany.xx>" >> internal_mailmap/.mailmap &&
@@ -220,6 +226,9 @@ test_expect_success 'Shortlog output (complex mapping)' '

 # git log with --pretty format which uses the name and email mailmap
placemarkers
 cat >expect <<\EOF
+Author Committed <committer@example.com> maps to Committed
<committer@example.com>
+Committer C O Mitter <committer@example.com> maps to Committed
<committer@example.com>
+
 Author C O Mitter <committer@example.com> maps to Committed
<committer@example.com>
 Committer C O Mitter <committer@example.com> maps to Committed
<committer@example.com>

@@ -260,6 +269,7 @@ OBJID (Santa Claus  DATE 5) five
 OBJID (Santa Claus  DATE 6) six
 OBJID (CTO          DATE 7) seven
 OBJID (Committed    DATE 8) eight
+OBJID (Committed    DATE 9) nine
 EOF
 test_expect_success 'Blame output (complex mapping)' '
        git blame one >actual &&

But that of course fails.

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

I most definitely did not sign this off, and I didn't add any of these
lines, nor wrote anything about this commit message.

It might be possible to simplify my patch "t: mailmap: add simple name
translation test" using the already existing "Committed
<committer@example.com>" mapping, but that most likely is going to
remove only one line, and would make the code less clear about what
that translation is trying to test.

Cheers.

-- 
Felipe Contreras