[Webkit-unassigned] [Bug 31340] Introduce a class representing HTML5 date&time values

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31340


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #43055|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #5 from David Levin <levin at chromium.org>  2009-11-16 10:53:34 PST ---
(From update of attachment 43055)
Thanks! Please consider addressing these final nits on landing.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog

This is a much better changelog!


> diff --git a/WebCore/html/ISODateTime.cpp b/WebCore/html/ISODateTime.cpp
> +// The oldest day of Gregorian Calendar is 1582-10-15.  We don't support dates older than it.

One space after periods.

> +static const int gregorianStartYear = 1582;
> +static const int gregorianStartMonth = 9; // 0-based October.

Use whole sentences in general. 0-based October doesn't make sense. It sound
like October is the 0th month. What you mean is that January is 0, so this
month 9 is really October.
Here's an idea for your comment  "// This is October, since months are 0
based."

>
> +static int dayOfWeek(int year, int month, int day)
> +{
> +    int shiftedMonth = month + 2;
> +    // 2:January, 3:Feburuary, 4:March, ...

Thanks! You had this before with the month++ and I missed it, but I find this
new form much easier to read.


> +// Very strict integer parser. Do not allow leading or trailing whitespaces unlike charactersToIntStrict().

s/whitespaces/whitespace/

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.