Subject: Re: [PATCHv2 27/56] string-list: Add API to remove an item from an unsorted list

From: Johannes Sixt <[email-masked]>
Date: Sat, 13 Aug 2011 11:08:17 +0200

Am 13.08.2011 00:14, schrieb Elijah Newren:
> On Fri, Aug 12, 2011 at 1:00 AM, Johannes Sixt <[email-masked]> wrote:
>> Am 8/12/2011 7:20, schrieb Elijah Newren:
>>> Here's an attempt for a delete_item API (note: only compile-tested).
>>
>> Seriously? You haven't even tested this patch, and still don't mark it
>> with RFC?
> ...
> However, I am unclear what you mean by not even testing the patch,
> though.  I couldn't find any unit-test harness or any other kind of
> testsuite for the string_list API.  I did review the code to make sure
> it looked right to me, added a use of your new function, and ran the
> standard testsuite in addition to my "re-merge all merges from
> git.git" testcase.  I even single stepped through the code in a
> debugger for good measure.  What testing did you want to see in
> particular?

This kind of testing is fine. And I appologize for asking a provocative
question. Of course, I know that you did test the new function suitably,
otherwise you wouldn't have submitted the series.

So, the question is rather, why did that sentence remain in the commit
message? The commit message should not be deceptive (and in particular
not blindly copy-pasted from an email that throws out a patch in the
hopes that somebody picks it up and massages into a good shape - I
thought it was clear that my patch was not a proper patch submission).

Think about someone browses history constrained by pathspec
'string-list.c'. This person will see this commit without any hint about
the merge-recursive series or that the new API is used in the next
commit, and will have to ask: "Why the heck is did someone introduce
this code and didn't even test it?"

-- Hannes