Re-reading my original review comment, I think it sounds more requiring than I intended it to be. My fault.
Also, this thread (and in particular Danny's last mail) nicely demonstrate another aspect of our patch submission process: namely, that patch reviews are nothing but the start of a discussion about the point they highlight, and it's fine and healthy to present a disagreeing opinion upon receiving a patch review. Daniel Julian Foad wrote on Tue, Nov 30, 2010 at 10:40:59 +0000: > On Mon, 2010-11-29, Danny Trebbien wrote: > [...] > > My conclusion from all of this is that regardless of the value of > > `repair`, my changes do not appear to decrease the performance of > > svn_subst_translate_string() as long as svn_subst_translate_string2() > > is called directly. > > Hi Danny. (I notice you changed your email "From" name to "Danny".) > > Statistics was never my strength so I'll just look to your conclusion. > It sounds like it doesn't need any optimization, certainly nothing > major. Therefore we should definitely make the functional change first. > > I just looked back at the previous emails and had a chat with Daniel > Sh., and he agrees. Would you like to re-post your patch, when you're > ready, without any of this optimization but with any other changes that > are still needed? > > Daniel Shahaf wrote: > > As I don't recall (m)any other issues with the patch, I think it's > > a short distance from resolving this issue to committing the patch. > > Yup, a short distance now. > > Thanks. > > - Julian > >