[webkit-reviews] review denied: [Bug 3477] some US-centric date formats not parsed by JavaScript (clock at news8austin.com) : [Attachment 2533] Improved patch

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Wed Jun 22 09:39:45 PDT 2005


Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 3477: some US-centric date formats not parsed by JavaScript (clock at
news8austin.com)
http://bugzilla.opendarwin.org/show_bug.cgi?id=3477

Attachment 2533: Improved patch
http://bugzilla.opendarwin.org/attachment.cgi?id=2533&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I'm not so happy with all the whitespace and indenting changes. There is lots
of code here that hasn't changed at all except that you seem to have run some
program's indenting on it (emacs, perhaps?).

Maybe we should do that in a separate pass, as it makes it harder to spot the
actual changes.

To me, some of the whitespace changes look wrong, too. Like the
strangely-indented body of DateProtoFuncImp::DateProtoFuncImp for example.

My (utc) comment above is about this code:

+    if ( (id == DateProtoFuncImp::ToGMTString) ||
+	  (id == DateProtoFuncImp::ToUTCString) )
+	 t = gmtime(&tv);
+    else if (id == DateProtoFuncImp::ToString)
+	 t = localtime(&tv);
+    else if (utc)
+	 t = gmtime(&tv);
+    else
+	 t = localtime(&tv);

I think the first two if statements are entirely unnecessary.

Because there were so many whitespace changes, I wasn't able to do a thorough
review of the new patch.



More information about the webkit-reviews mailing list