[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