Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:

>>> Look at the title:
>>> add 'git blame -e' tests
>>>
>>> s/blame/blame -e/
>>
>> And?  After copy/pasting this particular test with that substitution,
>> what do we get a test for?
>
> For 'git blame -e'.
>
>> What class of problem is it supposed to catch?
>
> Problems related to 'git blame -e'?

You very well know that we know you better than that, so it is no use to
pretend to be dumb.  It does not do anything other than wasting bandwidth
and irritating readers.

You know that you are not addressing "git blame with the -e option shows
wrong line numbers on its output".  The symptom you are checking with is
"e-mail address output from 'blame -e' used to add an extra '>' at the end
when only name is mapped, and I fixed it with the previous patch."

Why is it so hard to either

 (1) give the more descriptive answer upfront when somebody who did not
     read the first patch of the 3-patch series pointed it out the comment
     is not descriptive enough the first time; or

 (2) give the more descriptive answer and then say "we could do that, but
     when somebody views this change in "git log" as a part of 3-patch
     series, it should be clear enough---so let's aim for brevity instead
     of adding that two-line description" to defend the line in the patch?

> Or just apply it. Don't let the perfect be the enemy of the good.

Perfect is the enemy of the good, but it is not an excuse to be sloppy.

I tend to think that a single line "# blame -e" is sufficient if this were
a part of just a single patch that has the fix and the test to guard the
fix against future breakage (i.e. "not sloppy"). I would even say that not
even "# blame" is necessary in such a case.

But if this is a standalone patch to add this test, it does not describe
what it wants to test very well.

In any case, I have doubts that the fix should go to blame and not to
map_user(), so I'll see what happens in the further discussions.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]