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

restore Resolved Options table to ordering before PR #150 merged #165

Merged
merged 2 commits into from
Aug 28, 2023

Conversation

ben-allen
Copy link
Collaborator

fix #163

@ben-allen ben-allen marked this pull request as draft August 21, 2023 18:32
@anba
Copy link
Contributor

anba commented Aug 21, 2023

#163 (comment) has the expected order. That means only [[NumberingSystem]] and [[Locale]] need to be switched.

@ben-allen ben-allen force-pushed the restore-resolvedoptions-ordering branch from 3887a71 to 87eeb39 Compare August 21, 2023 21:35
@ben-allen ben-allen marked this pull request as ready for review August 21, 2023 21:35
@ben-allen ben-allen force-pushed the restore-resolvedoptions-ordering branch from 87eeb39 to b9609d6 Compare August 28, 2023 14:59
@FrankYFTang
Copy link
Collaborator

FrankYFTang commented Aug 28, 2023

#163 (comment) has the expected order. That means only [[NumberingSystem]] and [[Locale]] need to be switched.

This proposal reach Stage 3 in Oct 2021 and at that time the spec is
https://github.com/tc39/proposal-intl-duration-format/blob/6e1d1749473d92f7ff5f534e57d93b9a97d51286/spec.emu

the numberingSystem is the last in the table of that version.
And from my understanding, there are no consensus after that reach in TC39 to change the order of numberingSystem in resolvedOptions() yet. but somehow in the last merging, that order got accidentially changed and merged.

What I like to see is a "recover" PR to fix that technical mistake during merge.

I am open to discuss what should be the correct order and wiill likely to support a normative PR to put them in the right order as what you suggest, but that should be a different process in term of TC39.

Copy link
Collaborator

@FrankYFTang FrankYFTang left a comment

Choose a reason for hiding this comment

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

Thanks. This fix the accidential mistake of previous merge.

@FrankYFTang FrankYFTang merged commit f45eda4 into tc39:main Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

resolvedOptions order
3 participants