[webkit-reviews] review denied: [Bug 89963] [Platform] Implement Date Time format parser : [Attachment 150090] Patch 5
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jun 29 00:04:56 PDT 2012
Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 89963: [Platform] Implement Date Time format parser
https://bugs.webkit.org/show_bug.cgi?id=89963
Attachment 150090: Patch 5
https://bugs.webkit.org/attachment.cgi?id=150090&action=review
------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150090&action=review
> Source/WebCore/platform/text/DateTimeFormat.cpp:219
> + default:
> + LOG_ERROR("Invliad state=%d", state);
> + ASSERT_NOT_REACHED();
> + return false;
Please remove default: section. It's harmful.
> Source/WebCore/platform/text/DateTimeFormat.cpp:247
> + default:
> + LOG_ERROR("Invliad state=%d", state);
> + ASSERT_NOT_REACHED();
> + return false;
ditto.
Please add ASSERT_NOT_REACHED() at outside of the switch statement.
> Source/WebCore/platform/text/DateTimeFormat.h:100
> + virtual void visitField(FieldType, int) = 0;
Please add argument name for 'int', or add a comment for it.
> Source/WebKit/chromium/tests/DateTimeFormatTest.cpp:196
> + virtual void visitField(FieldType fieldType, int count)
add OVERRIDE
> Source/WebKit/chromium/tests/DateTimeFormatTest.cpp:201
> + virtual void visitLiteral(const String& string)
ditto.
More information about the webkit-reviews
mailing list