Normative: Sync the set of RoundingMode #2262
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #2262 +/- ##
===========================================
- Coverage 95.01% 81.77% -13.24%
===========================================
Files 20 17 -3
Lines 10803 10432 -371
Branches 1925 1476 -449
===========================================
- Hits 10264 8531 -1733
- Misses 503 1857 +1354
- Partials 36 44 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
This change requires a change to the documentation too. @ptomato do you want to land the spec change separately from the docs change, or should we combine into the same PR? To find the affected places, search for |
|
please review the change to the doc @sffc |
sffc
reviewed
Jun 10, 2022
justingrant
reviewed
Jun 10, 2022
ptomato
approved these changes
Jun 20, 2022
There was a problem hiding this comment.
The spec part seems correct. Does it need to be presented in July or is it part of the NFv3 proposal? Or can we consider it "pre-approved" since we already had written in the Temporal proposal when it went to Stage 3 that we intended to sync the rounding modes with NFv3?
I think there is a bit more work necessary for the documentation but I'd also be content to split that out into a different PR, together with updating the polyfill to support these rounding modes (we will need to write test262 for them)
|
ok, I feel a little bit overwhelm about the opinion of the .md changes. how about this- make suggestion of what exactly how I should revert/modify the .md and I commit those first. I currently just try to copy/paste the same set of documenat to different .md but I do see your point that in some condictions that might not be the best way to approach that. |
|
@FrankYFTang Sure, I'm also fine if you would prefer that @justingrant or I take it from here. I think we can continue by pushing to this branch ourselves. |
|
I think it needs a rebase - this seems to be branched off of a commit before RoundNumberToIncrementAsIfPositive was added. The list of rounding modes in the header of that operation also needs to be modified. |
ptomato
added
spec-text
|
@justingrant I made another attempt at the documentation, please have a look? |
justingrant
approved these changes
Jul 6, 2022
ptomato
added a commit
to ptomato/test262
that referenced
this pull request
Sep 9, 2022
…tring tests See tc39/proposal-temporal#2262, which reached consensus in the July 2022 TC39 meeting. This change added several rounding modes from the NumberFormat V3 proposal, some of which were listed as invalid in the roundingmode-invalid-string tests. Remove these items from the list of invalid modes, since they are no longer invalid.
ptomato
added a commit
to ptomato/test262
that referenced
this pull request
Sep 9, 2022
See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3. These tests use the same format as the previous ones. The tests for the "half" rounding modes aren't very good yet, as they don't show any of the differences between the tiebreaking schemes; there aren't any ties in the data to be broken. (Except in .toString().) A subsequent commit will correct this.
ptomato
added a commit
to ptomato/test262
that referenced
this pull request
Sep 9, 2022
The "half___" modes all round to the nearest increment except when there is a tie. The previous tests didn't test rounding in the case of any ties (except for .toString()) so here we use some different numbers in which there is a tie, in order to make tests where the "half___" modes are more thoroughly tested. See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3.
Ms2ger
pushed a commit
to ptomato/test262
that referenced
this pull request
Sep 21, 2022
…tring tests See tc39/proposal-temporal#2262, which reached consensus in the July 2022 TC39 meeting. This change added several rounding modes from the NumberFormat V3 proposal, some of which were listed as invalid in the roundingmode-invalid-string tests. Remove these items from the list of invalid modes, since they are no longer invalid.
Ms2ger
pushed a commit
to ptomato/test262
that referenced
this pull request
Sep 21, 2022
See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3. These tests use the same format as the previous ones. The tests for the "half" rounding modes aren't very good yet, as they don't show any of the differences between the tiebreaking schemes; there aren't any ties in the data to be broken. (Except in .toString().) A subsequent commit will correct this.
Ms2ger
pushed a commit
to ptomato/test262
that referenced
this pull request
Sep 21, 2022
The "half___" modes all round to the nearest increment except when there is a tie. The previous tests didn't test rounding in the case of any ties (except for .toString()) so here we use some different numbers in which there is a tie, in order to make tests where the "half___" modes are more thoroughly tested. See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3.
Ms2ger
pushed a commit
to tc39/test262
that referenced
this pull request
Sep 21, 2022
…tring tests See tc39/proposal-temporal#2262, which reached consensus in the July 2022 TC39 meeting. This change added several rounding modes from the NumberFormat V3 proposal, some of which were listed as invalid in the roundingmode-invalid-string tests. Remove these items from the list of invalid modes, since they are no longer invalid.
Ms2ger
pushed a commit
to tc39/test262
that referenced
this pull request
Sep 21, 2022
See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3. These tests use the same format as the previous ones. The tests for the "half" rounding modes aren't very good yet, as they don't show any of the differences between the tiebreaking schemes; there aren't any ties in the data to be broken. (Except in .toString().) A subsequent commit will correct this.
Ms2ger
pushed a commit
to tc39/test262
that referenced
this pull request
Sep 21, 2022
The "half___" modes all round to the nearest increment except when there is a tie. The previous tests didn't test rounding in the case of any ties (except for .toString()) so here we use some different numbers in which there is a tie, in order to make tests where the "half___" modes are more thoroughly tested. See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3.
GetUnsignedRoundingMode now includes additional RoundingModes expand, halfCeil, halfFloor, halfTrunc and halfEven. Sync spec text in RoundNumberToIncrement, ToTemporalRoundingMode, NegateTemporalRoundingMode to match it.
This brings the reference polyfill up to date with the new rounding modes. It adds a quick set of tests that only test the internal implementation, in order to make sure that the RoundNumberToIncrement operation behaves as intended. Full tests should test the observable outcome in test262.
Note that for times, several pairs of rounding modes are equivalent to each other (because times are never considered negative).
ptomato
force-pushed
the
roundingMode
branch
from
339ab3e to
edcb2e9
Compare
|
This achieved consensus at the July 2022 TC39 meeting, and I've merged it now that the test262 tests are merged. (The change would have caused the old tests to fail.) |
justingrant
reviewed
Sep 21, 2022
There was a problem hiding this comment.
Really top-notch work on the docs here. Amazingly clear explanation of really hard stuff. Thanks @ptomato!
catamorphism
pushed a commit
to catamorphism/test262
that referenced
this pull request
Oct 4, 2022
…tring tests See tc39/proposal-temporal#2262, which reached consensus in the July 2022 TC39 meeting. This change added several rounding modes from the NumberFormat V3 proposal, some of which were listed as invalid in the roundingmode-invalid-string tests. Remove these items from the list of invalid modes, since they are no longer invalid.
catamorphism
pushed a commit
to catamorphism/test262
that referenced
this pull request
Oct 4, 2022
See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3. These tests use the same format as the previous ones. The tests for the "half" rounding modes aren't very good yet, as they don't show any of the differences between the tiebreaking schemes; there aren't any ties in the data to be broken. (Except in .toString().) A subsequent commit will correct this.
catamorphism
pushed a commit
to catamorphism/test262
that referenced
this pull request
Oct 4, 2022
The "half___" modes all round to the nearest increment except when there is a tie. The previous tests didn't test rounding in the case of any ties (except for .toString()) so here we use some different numbers in which there is a tie, in order to make tests where the "half___" modes are more thoroughly tested. See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3.
catamorphism
pushed a commit
to catamorphism/test262
that referenced
this pull request
Oct 5, 2022
…tring tests See tc39/proposal-temporal#2262, which reached consensus in the July 2022 TC39 meeting. This change added several rounding modes from the NumberFormat V3 proposal, some of which were listed as invalid in the roundingmode-invalid-string tests. Remove these items from the list of invalid modes, since they are no longer invalid.
catamorphism
pushed a commit
to catamorphism/test262
that referenced
this pull request
Oct 5, 2022
See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3. These tests use the same format as the previous ones. The tests for the "half" rounding modes aren't very good yet, as they don't show any of the differences between the tiebreaking schemes; there aren't any ties in the data to be broken. (Except in .toString().) A subsequent commit will correct this.
catamorphism
pushed a commit
to catamorphism/test262
that referenced
this pull request
Oct 5, 2022
The "half___" modes all round to the nearest increment except when there is a tie. The previous tests didn't test rounding in the case of any ties (except for .toString()) so here we use some different numbers in which there is a tie, in order to make tests where the "half___" modes are more thoroughly tested. See tc39/proposal-temporal#2262 which added new rounding modes from NumberFormat V3.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
after the landing of #2207
GetUnsignedRoundingMode now includes additional RoundingModes
expand, halfCeil, halfFloor, halfTrunc and halfEven.
Sync spec text in RoundNumberToIncrement, ToTemporalRoundingMode,
NegateTemporalRoundingMode to match it.
@sffc @ptomato