Normative: Update calendar data consistency validation #2500

gibson042 · 2023-02-09T17:45:11Z

"era" and "eraYear" are still checked by PrepareTemporalFields in alphabetical order along with other fields when the calendar specifies their relevance, but ensuring both-or-neither and consistency between the pair and "year" is no longer part of the operation (although it remains part of CalendarResolveFields along with similar checks for "month" and "monthCode").

A followup should move Calendar-related operations from ECMA-402 to ECMA-262 after this and #2474... I initially included that here as 13e2c73, but backed it out to avoid potential merge conflicts.

TODO:

Update polyfill

codecov · 2023-02-09T17:51:31Z

Codecov Report

Merging #2500 (ecbe690) into main (1275241) will decrease coverage by 0.01%.
The diff coverage is 95.94%.

@@            Coverage Diff             @@
##             main    #2500      +/-   ##
==========================================
- Coverage   96.08%   96.07%   -0.01%     
==========================================
  Files          20       20              
  Lines       11600    11655      +55     
  Branches     2195     2201       +6     
==========================================
+ Hits        11146    11198      +52     
- Misses        390      393       +3     
  Partials       64       64

Files Changed	Coverage Δ
polyfill/lib/ecmascript.mjs	`98.42% <90.62%> (-0.05%)`	⬇️
polyfill/lib/calendar.mjs	`87.12% <100.00%> (+0.15%)`	⬆️
polyfill/lib/plainmonthday.mjs	`97.50% <100.00%> (ø)`
polyfill/lib/plainyearmonth.mjs	`98.27% <100.00%> (ø)`

ptomato

The editorial changes seem good in any case, could we get them in a separate PR?

In particular, "Editorial: Consolidate standard-field Calendar method logic into ECMA-262" is #1418, which I was planning on working on after getting all the normative issues finished. I realize it's later reverted in this PR, but it'd be great to put that work towards addressing that issue in a separate PR!

This looks to be less invasive than I thought it would be, so I'm more positive about it than I expected. It'd be helpful to me, if I could revisit it again once it's rebased.

spec/intl.html

spec/calendar.html

spec/abstractops.html

gibson042 · 2023-02-18T18:23:26Z

I rebased on main and extracted the initial sequence of editorial commits into #2506, upon which this PR now sits (diff). Ephemeral consolidation of Calendar method logic into ECMA-262 in 13e2c73 was lost in the process due to merge conflicts, so I'm explicitly mentioning it here.

spec/intl.html

This is an editorial change intended to reduce the diff of PR #2500, where this rename is part of the changes.

justingrant

I think this looks good, but I didn't spend enough time with it to be 100% confident so leaving for others to approve. A few minor comments below.

polyfill/lib/calendar.mjs

spec/calendar.html

justingrant · 2023-08-14T17:33:34Z

What's the plan with this PR? It's been open for a very long time. Is it still relevant?

ptomato · 2023-08-14T18:47:30Z

What's the plan with this PR? It's been open for a very long time. Is it still relevant?

It's waiting on resolution of #2500 (comment)

justingrant · 2023-08-14T21:47:06Z

What's the plan with this PR? It's been open for a very long time. Is it still relevant?

It's waiting on resolution of #2500 (comment)

@gibson042, possible to respond to that comment so we can unblock this? Thx!

gibson042 · 2023-08-15T23:22:09Z

Updated and rebased onto main.

spec/calendar.html

ptomato

Works for me, I'm fine to merge it along with tc39/test262#3835 when other reviewers are satisfied.

polyfill/lib/calendar.mjs

gibson042 · 2023-08-17T15:59:42Z

Rebased.

Aditi-1400

Looks good to me! :)

Fixes tc39#2497

"era" and "eraYear" are still checked in alphabetical order along with other fields when the calendar specifies their relevance, but ensuring both-or-neither and consistency between the pair and "year" is no longer part of the operation (although it remains part of CalendarResolveFields along with similar checks for "month" and "monthCode"). Fixes tc39#2465

…method

…ds` algorithm Keep it in the implementation-defined CalendarFieldDescriptors operation

As of tc39/proposal-temporal#2500 , year is always optional for the ISO 8601 calendar.

gibson042 requested a review from ptomato February 9, 2023 17:45

ptomato reviewed Feb 16, 2023

View reviewed changes

spec/intl.html Show resolved Hide resolved

spec/calendar.html Outdated Show resolved Hide resolved

spec/abstractops.html Outdated Show resolved Hide resolved

gibson042 force-pushed the gh-2466-calendar-field-validation branch from a1af46b to 7d1666d Compare February 18, 2023 18:10

gibson042 mentioned this pull request Feb 18, 2023

Editorial: Improve alignment between ISO 8601 vs. other-calendar operations #2506

Merged

gibson042 force-pushed the gh-2466-calendar-field-validation branch 2 times, most recently from 22f90a3 to a05869c Compare February 19, 2023 18:04

gibson042 force-pushed the gh-2466-calendar-field-validation branch from a05869c to 8b967f1 Compare February 26, 2023 17:27

ptomato force-pushed the gh-2466-calendar-field-validation branch from 8b967f1 to 0fd6708 Compare February 27, 2023 18:31

ptomato force-pushed the gh-2466-calendar-field-validation branch from 0fd6708 to 28b99a6 Compare April 18, 2023 19:38

gibson042 force-pushed the gh-2466-calendar-field-validation branch from 28b99a6 to e5fc09d Compare April 19, 2023 14:57

Aditi-1400 reviewed May 3, 2023

View reviewed changes

spec/intl.html Outdated Show resolved Hide resolved

ptomato added a commit that referenced this pull request May 18, 2023

Editorial: Rename ResolveISOMonth → ISOResolveMonth

e37bbf4

This is an editorial change intended to reduce the diff of PR #2500, where this rename is part of the changes.

ptomato added a commit that referenced this pull request May 19, 2023

Editorial: Rename ResolveISOMonth → ISOResolveMonth

53a7f2d

This is an editorial change intended to reduce the diff of PR #2500, where this rename is part of the changes.

ptomato mentioned this pull request May 19, 2023

Editorial changes #2577

Merged

ptomato force-pushed the gh-2466-calendar-field-validation branch from e5fc09d to 5580db6 Compare May 19, 2023 17:48

This was referenced May 23, 2023

Add polyfill changes for calendar data consistency validation #2585

Closed

Temporal: Update test for monthDayFromFields() tc39/test262#3835

Merged

justingrant reviewed Jun 1, 2023

View reviewed changes

polyfill/lib/calendar.mjs Outdated Show resolved Hide resolved

spec/calendar.html Outdated Show resolved Hide resolved

ptomato mentioned this pull request Jul 13, 2023

Final normative spec text plan #2628

Closed

91 tasks

gibson042 force-pushed the gh-2466-calendar-field-validation branch 2 times, most recently from b01d958 to 47a285b Compare August 15, 2023 23:20

gibson042 mentioned this pull request Aug 16, 2023

Polyfill: Be more robust against late-run primordial manipulation #2650

Merged

gibson042 requested review from Aditi-1400 and ptomato August 16, 2023 15:20

gibson042 requested a review from justingrant August 16, 2023 15:20

justingrant reviewed Aug 16, 2023

View reviewed changes

spec/calendar.html Outdated Show resolved Hide resolved

ptomato approved these changes Aug 16, 2023

View reviewed changes

polyfill/lib/calendar.mjs Outdated Show resolved Hide resolved

gibson042 force-pushed the gh-2466-calendar-field-validation branch from 47a285b to bf1158f Compare August 17, 2023 15:59

gibson042 force-pushed the gh-2466-calendar-field-validation branch from bf1158f to 452d115 Compare August 17, 2023 16:06

Aditi-1400 approved these changes Aug 21, 2023

View reviewed changes

gibson042 and others added 7 commits August 21, 2023 09:18

Normative: Isolate ISO 8601 month consistency validation

d0f60dc

Fixes tc39#2497

Editorial: Relax assertion to account for timeZone: object

170b35e

Normative: Determine Temporal type in the ECMA-402 Calendar fields …

a25808e

…method

Polyfill: Align with spec

e37fd00

Normative: Remove type determination from the ECMA-402 Calendar `fiel…

13e30b6

…ds` algorithm Keep it in the implementation-defined CalendarFieldDescriptors operation

Update test262

ecbe690

ptomato force-pushed the gh-2466-calendar-field-validation branch from 452d115 to ecbe690 Compare August 21, 2023 16:23

ptomato merged commit 4345a8c into tc39:main Aug 21, 2023
9 checks passed

gibson042 added a commit to gibson042/test262 that referenced this pull request Aug 21, 2023

Temporal: Update PlainMonthDay-related tests

1574f92

As of tc39/proposal-temporal#2500 , year is always optional for the ISO 8601 calendar.

This was referenced Aug 21, 2023

Temporal: Update PlainMonthDay-related tests tc39/test262#3899

Merged

Polyfill: Update per #2500 #2663

Merged

Where should month without year or monthCode be rejected? #2664

Closed

ptomato pushed a commit to gibson042/test262 that referenced this pull request Sep 19, 2023

Temporal: Update PlainMonthDay-related tests

21056b2

As of tc39/proposal-temporal#2500 , year is always optional for the ISO 8601 calendar.

ptomato pushed a commit to gibson042/test262 that referenced this pull request Sep 28, 2023

Temporal: Update PlainMonthDay-related tests

a616c08

As of tc39/proposal-temporal#2500 , year is always optional for the ISO 8601 calendar.

gibson042 added a commit to gibson042/test262 that referenced this pull request Oct 26, 2023

Temporal: Update PlainMonthDay-related tests

c42f75c

As of tc39/proposal-temporal#2500 , year is always optional for the ISO 8601 calendar.

gibson042 added a commit to tc39/test262 that referenced this pull request Oct 26, 2023

Temporal: Update PlainMonthDay-related tests

5c62968

As of tc39/proposal-temporal#2500 , year is always optional for the ISO 8601 calendar.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Update calendar data consistency validation #2500

Normative: Update calendar data consistency validation #2500

gibson042 commented Feb 9, 2023 •

edited

Loading

codecov bot commented Feb 9, 2023 •

edited

Loading

ptomato left a comment

gibson042 commented Feb 18, 2023

justingrant left a comment

justingrant commented Aug 14, 2023

ptomato commented Aug 14, 2023

justingrant commented Aug 14, 2023

gibson042 commented Aug 15, 2023

ptomato left a comment

gibson042 commented Aug 17, 2023

Aditi-1400 left a comment

Normative: Update calendar data consistency validation #2500

Normative: Update calendar data consistency validation #2500

Conversation

gibson042 commented Feb 9, 2023 • edited Loading

codecov bot commented Feb 9, 2023 • edited Loading

Codecov Report

ptomato left a comment

Choose a reason for hiding this comment

gibson042 commented Feb 18, 2023

justingrant left a comment

Choose a reason for hiding this comment

justingrant commented Aug 14, 2023

ptomato commented Aug 14, 2023

justingrant commented Aug 14, 2023

gibson042 commented Aug 15, 2023

ptomato left a comment

Choose a reason for hiding this comment

gibson042 commented Aug 17, 2023

Aditi-1400 left a comment

Choose a reason for hiding this comment

gibson042 commented Feb 9, 2023 •

edited

Loading

codecov bot commented Feb 9, 2023 •

edited

Loading