[webkit-reviews] review denied: [Bug 30187] Implicit AtomicString(char*) considered harmful : [Attachment 44270] patch 2 (stylistic fixes)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 28 18:09:00 PST 2009


Maciej Stachowiak <mjs at apple.com> has denied Jens Alfke <snej at chromium.org>'s
request for review:
Bug 30187: Implicit AtomicString(char*) considered harmful
https://bugs.webkit.org/show_bug.cgi?id=30187

Attachment 44270: patch 2 (stylistic fixes)
https://bugs.webkit.org/attachment.cgi?id=44270&action=review

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
This patch is kind of large and combines multiple logically separate changes. I
think it would be better to split it up into the following independent
components:

1) The HashMap changes. (Yeah, it sucks that they are not unit tested but they
seem pretty separate from the rest).
2) One or more patches removing dependence of various pieces of code on
implicit conversion from char* to AtomicString.
3) Final patch to make AtomicString(char*) explicit.

r- to consider such a split. But if that's impractical, just go ahead and
reflag this patch for review.


More information about the webkit-reviews mailing list