[webkit-reviews] review denied: [Bug 47664] Replace lots of String::format() usages by StringConcatenate : [Attachment 70744] Patch v2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 14 11:16:47 PDT 2010


Gavin Barraclough <barraclough at apple.com> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 47664: Replace lots of String::format() usages by StringConcatenate
https://bugs.webkit.org/show_bug.cgi?id=47664

Attachment 70744: Patch v2
https://bugs.webkit.org/attachment.cgi?id=70744&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
Looks great, but one tiny problem.
Our default char->UChar conversion is simple latin1, which is to say
zero-extend.  You should change 'char m_buffer;' to 'unsigned char m_buffer;'
in the adapter class.
Do you know what accounted for the change in layout test results? - might this
be related?

Otherwise all looks good.  At some point in the future it might be nice rename
makeString() to String::cat() – I'm not sure, but I think I like the idea of
tying it to the class to implicitly indicate that the return type is String –
but sticking with the same name from JSC is a fine decision for now, and that
would be an easy find & replace later if we do want to do so.


More information about the webkit-reviews mailing list