[webkit-reviews] review denied: [Bug 23477] Support for WCSS extensions : [Attachment 30901] Updated Patch for WCSS Inputformat
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Aug 7 09:21:08 PDT 2009
Eric Seidel <eric at webkit.org> has denied Sreedhar Vaddi
<sreedhar.vaddi at nokia.com>'s request for review:
Bug 23477: Support for WCSS extensions
https://bugs.webkit.org/show_bug.cgi?id=23477
Attachment 30901: Updated Patch for WCSS Inputformat
https://bugs.webkit.org/attachment.cgi?id=30901&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
Lots of style errors. Please run check-webkit-style.
+ MaskBase* m = NULL;
No need for ()
+ return (validate(text, eb));
Style:
+bool TextFormatMask::createStaticMask(const WebCore::String &p, int &pos)
eb is not a good variable name:
+bool TextFormatMask::validate(const String& text, ErrorBlock& eb)
No == 0:
+ if (text.length() == 0)
Style:
+ ErrorBlock() : m_start(-1), m_extent(-1) {}
+ struct ErrorBlock
+ {
Style:
+ InputFormatMaskType nextInputMaskType(WebCore::Frame *frame, int aOffset);
overused WebCore:: namespace.
Please run check-webkit-style and have someone like George clear this for style
before re-posting. Silly to waste reviewers time fixing style issues.
I'm not sure we want this patch from a technical perspective (being discussed
on webkit-dev), but we certainly can't accept it as-is style-wise.
You'll also have more luck getting reviews by posting smaller patches.
svn-create-patch will correctly put your layout tests and ChangeLogs first in
the patch file which can make for easier reviews, but is not required.
More information about the webkit-reviews
mailing list