[webkit-reviews] review denied: [Bug 89963] [Platform] Implement Date Time format parser : [Attachment 150090] Patch 5

[bugzilla-daemon at webkit.org]

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.