-
Notifications
You must be signed in to change notification settings - Fork 159
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
toString() precision #1006
toString() precision #1006
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1006 +/- ##
==========================================
+ Coverage 94.33% 94.38% +0.05%
==========================================
Files 18 18
Lines 6420 6556 +136
Branches 959 983 +24
==========================================
+ Hits 6056 6188 +132
- Misses 357 361 +4
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
f3f9928
to
40a11a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I rebased and fixed the failing test262 tests
dad50c0
to
256206b
Compare
256206b
to
afafce9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo the one comment
In toString() we are going to have a different default rounding mode than in round() and difference(), so don't hardcode the default in the abstract operation. See: #329
This will be reused in Temporal.DateTime.prototype.toString when we add precision options. See: #329
This will be reused in Temporal.Instant.prototype.toString when we add precision options. See: #329
On DateTime.toString(), Time.toString(), and Instant.toString(), we add an options bag with the options `fractionalSecondDigits`, `smallestUnit`, and `roundingMode`. `smallestUnit` and `fractionalSecondDigits` control which units are output, and `roundingMode` is a rarely-needed option that allows other rounding behaviour than truncation. The default behaviour is changed as well. Previously seconds would not be output if the seconds and lower components would be 0, and the precision after the decimal point would be 0, 3, 6, or 9. After this change, seconds are always output, and all trailing zeroes after the decimal point are dropped. This change requires a lot of adjustments to expected test output. Closes: #329
afafce9
to
33728dd
Compare
Addressed the review comments, so merging as per the approval |
The bulk of this commit is adding
:00
to expected test output; it's probably safe to ignore those parts when reviewing this.Closes: #329