400k is too large of a patch for anyone to review. I would suggest you start by splitting out the layout test changes from the rest of the patch. I would also suggest that you try to post the code changes in smaller chunks. Ideal patch review size is <20k, but that's not always possible for feature patches of course. :)
-eric On Thu, Jun 25, 2009 at 2:32 AM, Roland Steiner<[email protected]> wrote: > Hi Dave, > > thanks again for the feedback! I've now submitted a patch to bug #3749 with > a basic ruby implementation with all the changes discussed on the list. > (including the flag). Would be great if you could take time to review the > patch whenever you can spare the time. > > Cheers, > > Roland > > On Tue, Jun 23, 2009 at 2:12 AM, David Hyatt <[email protected]> wrote: >> >> On Jun 21, 2009, at 11:18 PM, Roland Steiner wrote: >> >>> Hi Dave, >>> >>> as I will probably need to special-case height() for ruby InlineBox >>> objects in the same way as is done for SVG boxes (still ironing out the >>> details, though), making height() virtual was exactly my intent. I would >>> have thought that the performance cost of a virtual call to height() would >>> be offset by being able to remove the isSVG() condition inside (and later a >>> potential isRuby() condition as well). >>> >>> Now if there are actual performance reasons for that bit and/or for >>> having height() be non-virtual, then I may need to find another solution. >>> >>> Thanks, >>> >>> Roland >> >> You could probably just rename the m_isSVG bit to be something like >> m_calculatesHeight, and then the virtual method that height() calls when >> that is true could be renamed to be more general. >> >> dave >> ([email protected]) >> > > _______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
