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.

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

Normative: Fix inconsistent property names/order returned by getISOFields() #1510

Merged
merged 2 commits into from
May 25, 2021
Merged

Normative: Fix inconsistent property names/order returned by getISOFields() #1510

merged 2 commits into from
May 25, 2021

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented May 12, 2021

Fixes the names of properties on the return value from ZonedDateTime.getISOFields, which were missing the iso prefix.

Fixes the definition order of properties on the return value from PlainDateTime.getISOFields, which was incorrectly alphabetized.

Adds test262 tests for these fixes to all getISOFields methods.

Closes: #1508

@ptomato
Normative: Correct names of properties on ZonedDateTime.getISOFields …
d4c90c8 
…object

This was an oversight due to two pull requests crossing each other. The
"iso" prefix was an intentional choice, for consistency, and to keep the
possibility of time calendars open.

Closes: #1508
@ptomato
Normative: Correct definition order of properties on PlainDateTime.ge…
ea55c7d 
…tISOFields object

This was intended to be alphabetical.

See: #1508
@ptomato ptomato requested a review from Ms2ger May 12, 2021 22:34
@ptomato ptomato marked this pull request as draft May 12, 2021 22:35
@codecov
Copy link

codecov bot commented May 12, 2021 •
edited
Loading

Codecov Report

Merging #1510 (ea55c7d) into main (2c44028) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1510   +/-   ##
=======================================
  Coverage   95.58%   95.58%           
=======================================
  Files          19       19           
  Lines       11139    11139           
  Branches     1736     1736           
=======================================
  Hits        10647    10647           
  Misses        486      486           
  Partials        6        6
Flag Coverage Δ
test262 61.11% <ø> (+0.08%) ⬆️
tests 91.76% <ø> (ø)

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

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c44028...ea55c7d. Read the comment docs.

@ptomato
Copy link
Collaborator Author

ptomato commented May 12, 2021

The test262 tests turned out to be pretty quick, so I included them already.

Marked as draft to avoid merging without TC39 consensus.

@ptomato ptomato added the needs plenary input Needs to be presented to the committee and feedback incorporated label May 14, 2021
@justingrant justingrant changed the title Normative: Fix properties on objects returned from getISOFields() Normative: Fix properties on objects returned from ZonedDateTime.p.getISOFields() May 15, 2021
@justingrant justingrant changed the title Normative: Fix properties on objects returned from ZonedDateTime.p.getISOFields() Normative: Fix property names returned from ZonedDateTime.p.getISOFields() May 15, 2021
@justingrant justingrant changed the title Normative: Fix property names returned from ZonedDateTime.p.getISOFields() Normative: Fix property names returned from ZonedDateTime & PlainDateTime getISOFields() methods May 18, 2021
@justingrant justingrant changed the title Normative: Fix property names returned from ZonedDateTime & PlainDateTime getISOFields() methods Normative: Fix property names & order returned from ZonedDateTime/PlainDateTime getISOFields() methods May 18, 2021
@justingrant justingrant changed the title Normative: Fix property names & order returned from ZonedDateTime/PlainDateTime getISOFields() methods Normative: Fix inconsistent property names/order returned from getISOFields() May 18, 2021
@justingrant justingrant changed the title Normative: Fix inconsistent property names/order returned from getISOFields() Normative: Fix inconsistent property names/order returned by getISOFields() May 18, 2021
@ptomato ptomato marked this pull request as ready for review May 25, 2021 22:45
@ptomato
Copy link
Collaborator Author

ptomato commented May 25, 2021

Achieved consensus in today's TC39 meeting.

@ptomato ptomato merged commit 45628b5 into main May 25, 2021
@ptomato ptomato deleted the 1508-fix-zoneddatetime-getisofields branch May 25, 2021 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ms2ger Ms2ger Ms2ger approved these changes

Assignees
No one assigned
Labels
needs plenary input Needs to be presented to the committee and feedback incorporated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect property names in specification for Temporal.ZonedDateTime.prototype.getISOFields
2 participants
@ptomato @Ms2ger