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

use all values max and min fraction and significant digits #3405

Merged
merged 1 commit into from
Mar 14, 2022
Merged

use all values max and min fraction and significant digits #3405

merged 1 commit into from
Mar 14, 2022

Conversation

romulocintra
Copy link
Member

Summary :

WIP updating #3364

Will be draft PR depending on: tc39/proposal-intl-numberformat-v3#81 that could also affect the tests

@rwaldron
Copy link
Contributor

LGTM, ping when out of draft mode

@romulocintra romulocintra marked this pull request as ready for review February 18, 2022 15:10
@romulocintra
Copy link
Member Author

LGTM, ping when out of draft mode

Thanks for having a look at it, will leave it like this and do incremental coverage as it covers #3364

@romulocintra romulocintra added the ECMA-402 ECMA-402 related PRs label Feb 21, 2022
@romulocintra romulocintra requested a review from rwaldron March 2, 2022 16:59
ptomato
ptomato approved these changes Mar 11, 2022
View reviewed changes
Copy link
Contributor

@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.

This test seems correct according to https://tc39.es/proposal-intl-numberformat-v3/out/numberformat/proposed.html#sec-setnfdigitoptions

According to #3364 the idea was to add tests for different values of roundingPriority. Is that still planned to be added in another PR?

@romulocintra
Copy link
Member Author

This test seems correct according to https://tc39.es/proposal-intl-numberformat-v3/out/numberformat/proposed.html#sec-setnfdigitoptions

According to #3364 the idea was to add tests for different values of roundingPriority. Is that still planned to be added in another PR?

Yes that's the idea add those in a new PR

@ptomato
Copy link
Contributor

ptomato commented Mar 14, 2022

OK, thanks!

@ptomato ptomato merged commit 81895b1 into tc39:main Mar 14, 2022
@romulocintra romulocintra mentioned this pull request Mar 16, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sffc sffc sffc approved these changes

@ptomato ptomato ptomato approved these changes

@rwaldron rwaldron Awaiting requested review from rwaldron

Assignees
No one assigned
Labels
ECMA-402 ECMA-402 related PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@romulocintra @rwaldron @ptomato @sffc