Skip to content

Update parseISO.baseline for PR #4318 #4571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
chakrabot merged 1 commit into from
Jan 18, 2018
Merged

Update parseISO.baseline for PR #4318 #4571

chakrabot merged 1 commit into from
Jan 18, 2018

Conversation

@xiaoyinl
Copy link
Contributor

@xiaoyinl xiaoyinl commented Jan 18, 2018

Sorry I forgot to update parseISO.baseline again for the latest Date.parse changes in PR #4318.

Related #4543

@xiaoyinl
Update parseISO.baseline for PR #4318
ea7f37e 
PR #4318 updates how Date.parse handles padded years, but parseISO.baseline
wasn't updated to reflect this change, causing some test failures.
@dilijev dilijev self-assigned this Jan 18, 2018
@dilijev
Copy link
Contributor

dilijev commented Jan 18, 2018

Ah, thanks. Would have missed this until the next nightly bug landed on my plate :)

dilijev
dilijev approved these changes Jan 18, 2018
View reviewed changes
Copy link
Contributor

@dilijev dilijev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dilijev
Copy link
Contributor

dilijev commented Jan 18, 2018

I was wondering about these lines when I last updated the baseline -- what I found was that we and d8 both do the unintuitive thing and the others just fail:

## Source
print(new Date(Date.parse('0001-02-03Z')).toISOString())

┌──────────┬──────────────────────────┐
│ jsvu-ch  │ 2003-01-02T00:00:00.000Z │
│ jsvu-d8  │                          │
├──────────┼──────────────────────────┤
│ jsvu-sm  │                          │
│          │ RangeError: invalid date │
├──────────┼──────────────────────────┤
│ jsvu-jsc │                          │
│          │ RangeError: Invalid Date │
└──────────┴──────────────────────────┘

This new behavior certainly makes more sense, especially from the perspective of treating strings that match /\d{4}-\d{2}-\d{2}/ as an ISO date format.

@chakrabot chakrabot merged commit ea7f37e into chakra-core:release/1.8 Jan 18, 2018
chakrabot pushed a commit that referenced this pull request Jan 18, 2018
@dilijev
[MERGE #4571 @xiaoyinl] Update parseISO.baseline for PR #4318
6734b00 
Merge pull request #4571 from xiaoyinl:isobaseline

Sorry I forgot to update parseISO.baseline again for the latest Date.parse changes in PR #4318.

Related #4543
chakrabot pushed a commit that referenced this pull request Jan 18, 2018
@dilijev
[1.8>1.9] [MERGE #4571 @xiaoyinl] Update parseISO.baseline for PR #4318
7749fb8 
Merge pull request #4571 from xiaoyinl:isobaseline

Sorry I forgot to update parseISO.baseline again for the latest Date.parse changes in PR #4318.

Related #4543
chakrabot pushed a commit that referenced this pull request Jan 18, 2018
@dilijev
[1.9>master] [1.8>1.9] [MERGE #4571 @xiaoyinl] Update parseISO.baseli…
fe4ba12 
…ne for PR #4318

Merge pull request #4571 from xiaoyinl:isobaseline

Sorry I forgot to update parseISO.baseline again for the latest Date.parse changes in PR #4318.

Related #4543
@xiaoyinl xiaoyinl deleted the isobaseline branch January 20, 2018 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@dilijev dilijev dilijev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dilijev dilijev

Labels

External Contributor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xiaoyinl @dilijev @chakrabot