Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(26)

Issue 1229903004: Make dates default to the local timezone if none specified (Closed)

Created:
4 years, 2 months ago by hichris123
Modified:
4 years, 2 months ago
Reviewers:
ulan
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Make dates default to the local timezone if none specified In ES5, dates were supposed to default to UTC if no timezone was specified. However, this changed in ES6, which specified that dates should be in the local timezone if no timezone was specified. This CL updates our behavior to match that part of the ES6 spec. BUG=chromium:391730, v8:4242 LOG=Y Committed: https://crrev.com/f06754a8e1d305a43560705f6c167d85d40e602d Cr-Commit-Position: refs/heads/master@{#29854}

Patch Set 1 #

Patch Set 2 : Fix merge issues #

Patch Set 3 : Update tests #

Patch Set 4 : Fix test #

Total comments: 4

Patch Set 5 : Update comments/method names #

Patch Set 6 : Update test #

Patch Set 7 : Actually fix test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -76 lines) Patch
M src/dateparser.h View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M src/dateparser-inl.h View 1 2 3 4 6 chunks +10 lines, -14 lines 0 comments Download
M test/mjsunit/date.js View 1 2 3 1 chunk +54 lines, -54 lines 0 comments Download
M test/mjsunit/date-parse.js View 1 2 3 4 5 6 1 chunk +3 lines, -3 lines 0 comments Download
M test/test262-es6/test262-es6.status View 1 2 1 chunk +0 lines, -3 lines 0 comments Download
M test/test262/test262.status View 1 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (3 generated)
ulan
lgtm, thanks! https://codereview.chromium.org/1229903004/diff/60001/src/dateparser-inl.h File src/dateparser-inl.h (right): https://codereview.chromium.org/1229903004/diff/60001/src/dateparser-inl.h#newcode25 src/dateparser-inl.h:25: // Accept ES5 ISO 8601 date-time-strings or ...
4 years, 2 months ago (2015-07-24 13:56:25 UTC) #2
hichris123
On 2015/07/24 13:56:25, ulan wrote: > lgtm, thanks! > > https://codereview.chromium.org/1229903004/diff/60001/src/dateparser-inl.h > File src/dateparser-inl.h (right): ...
4 years, 2 months ago (2015-07-24 14:23:16 UTC) #3
ulan
> One quick note -- I'll address the comments in a bit -- the method ...
4 years, 2 months ago (2015-07-24 14:32:48 UTC) #4
hichris123
Alright, I think I updated everything to reflect the new behavior and that it's using ...
4 years, 2 months ago (2015-07-24 15:10:04 UTC) #5
ulan
On 2015/07/24 15:10:04, hichris123 wrote: > Alright, I think I updated everything to reflect the ...
4 years, 2 months ago (2015-07-24 15:20:42 UTC) #6
hichris123
On 2015/07/24 15:20:42, ulan wrote: > Looks good. Needed to fix one test (not sure ...
4 years, 2 months ago (2015-07-24 15:28:30 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1229903004/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1229903004/120001
4 years, 2 months ago (2015-07-24 16:54:46 UTC) #10
commit-bot: I haz the power
Committed patchset #7 (id:120001)
4 years, 2 months ago (2015-07-24 17:19:39 UTC) #11
commit-bot: I haz the power
4 years, 2 months ago (2015-07-24 17:19:59 UTC) #12
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/f06754a8e1d305a43560705f6c167d85d40e602d
Cr-Commit-Position: refs/heads/master@{#29854}

Powered by Google App Engine
This is Rietveld 408576698