Am 13.08.2011 00:14, schrieb Elijah Newren: > On Fri, Aug 12, 2011 at 1:00 AM, Johannes Sixt <j.sixt@xxxxxxxxxxxxx> 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 -- 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