Skip to content
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.

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

Normative: Avoid ToString(calendar) when the calendar-id is unused #2269

Merged
merged 4 commits into from
Aug 3, 2022

Conversation

anba
Copy link
Contributor

@anba anba commented Jun 9, 2022

TemporalZonedDateTimeToString doesn't call ToString(timeZone) when
showTimeZone == "never". Do the same for ToString(calendar).

TemporalYearMonthToString and TemporalMonthDayToString weren't
changed because both have to special-case non-ISO-8601 calendars.

@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #2269 (2ae54ff) into main (08d1359) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2269   +/-   ##
=======================================
  Coverage   90.95%   90.95%           
=======================================
  Files          19       19           
  Lines       10465    10466    +1     
  Branches     1678     1680    +2     
=======================================
+ Hits         9518     9519    +1     
  Misses        939      939           
  Partials        8        8           
Flag Coverage Δ
test262 85.15% <100.00%> (-0.01%) ⬇️
tests 81.47% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
polyfill/lib/ecmascript.mjs 92.95% <100.00%> (+<0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@anba anba mentioned this pull request Jun 17, 2022
Copy link
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

Makes sense, for consistency. I will mark this as draft until presented to TC39.

@ptomato ptomato marked this pull request as draft June 30, 2022 10:07
@ptomato ptomato added spec-text Specification text involved normative Would be a normative change to the proposal labels Jul 4, 2022
ptomato added a commit to ptomato/test262 that referenced this pull request Jul 29, 2022
…lendar)

This implements the normative change in
tc39/proposal-temporal#2269 which reached
consensus at the July 2022 TC39 meeting.

There was already a test for PlainDate for this topic, which needs to be
adjusted to accommodate the normative change. Tests for PlainDateTime and
ZonedDateTime did not yet exist, so add new ones based on the PlainDate
test.
@ptomato ptomato force-pushed the calendar-to-string branch from 70088a4 to 1a0e382 Compare July 29, 2022 22:35
@ptomato
Copy link
Collaborator

ptomato commented Jul 29, 2022

This change achieved consensus in the July 2022 TC39 meeting. I've added another commit to bring the reference code in sync with the spec change. Since that will break tests, we should wait until tc39/test262#3623 is merged and update the test262 submodule commit as part of merging this.

spec/zoneddatetime.html Outdated Show resolved Hide resolved
@ptomato ptomato force-pushed the calendar-to-string branch from 1a0e382 to a5a8186 Compare August 2, 2022 15:25
Ms2ger pushed a commit to tc39/test262 that referenced this pull request Aug 3, 2022
…lendar)

This implements the normative change in
tc39/proposal-temporal#2269 which reached
consensus at the July 2022 TC39 meeting.

There was already a test for PlainDate for this topic, which needs to be
adjusted to accommodate the normative change. Tests for PlainDateTime and
ZonedDateTime did not yet exist, so add new ones based on the PlainDate
test.
@Ms2ger
Copy link
Collaborator

Ms2ger commented Aug 3, 2022

tc39/test262#3623 is merged

anba and others added 4 commits August 3, 2022 10:14
`TemporalZonedDateTimeToString` doesn't call `ToString(timeZone)` when
`showTimeZone == "never"`. Do the same for `ToString(calendar)`.

`TemporalYearMonthToString` and `TemporalMonthDayToString` weren't
changed because both have to special-case non-ISO-8601 calendars.
The repeated normative change in the previous commit goes into its own
abstract operation, MaybeFormatCalendarAnnotation.
While we are at it, updates FormatCalendarAnnotation to use a structured
header, and updates the link in the editor's note.
Implements the normative change in the previous commit in the polyfill,
for the purpose of verifying test262 tests.
@ptomato ptomato force-pushed the calendar-to-string branch from a5a8186 to 2ae54ff Compare August 3, 2022 17:16
@ptomato ptomato marked this pull request as ready for review August 3, 2022 17:16
@ptomato ptomato merged commit 59b1bff into tc39:main Aug 3, 2022
@anba anba deleted the calendar-to-string branch August 4, 2023 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
normative Would be a normative change to the proposal spec-text Specification text involved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants