Re: [PATCH v3 0/2] test: tests for the "double > from mailmap" bug

On Tue, Feb 14, 2012 at 11:14 PM, Jeff King [off-list ref] wrote:

My general review process is (in this order):

 1. Figure out why a change is needed. This should be explained in the
    commit message. And no, just adding tests is not assumed to be
    needed.  Why did the old tests not cover this case?

As I already mentioned more than once; the first patch is not related
to any fix.

    The answer can
    be a simple "nobody bothered to write them", and that's OK.

 That can be derived from the word "add". You can't add something that
is already there.

    But
    some description of the current state can help reviewers understand
    the rationale (e.g., "we tested with shortlog, but not mailmap, and
    certain code paths are only exposed through mailmap").

You are assuming too much. That's not the purpose, that's why I didn't menti

 2. Figure out what the change should be doing.

t: mailmap: add 'git blame -e' tests

That's what the change should be doing; nothing more, nothing less.

I wonder why you have to assume the worst. When I see a commit message
like that, I assume that there were no previous tests for that (thus
the word 'add'), and that's all I need to know.

If you want to extrude meaning from where there isn't, well, go ahead,
but there's nothing else really.

Cheers.

-- 
Felipe Contreras