-
Notifications
You must be signed in to change notification settings - Fork 149
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
Normative: Change offset in ISODateTimeWithinLimits to actually 24h #1723
Normative: Change offset in ISODateTimeWithinLimits to actually 24h #1723
Conversation
The note (and the algorithm itself) claims: > time within 24 hours (8.64 × 10^16 nanoseconds) This is incorrect, 24 hours are 8.64 × 10^13 nanoseconds, which is the value that was intended to be used here (it's already correctly used in other places). 8.64 × 10^16 nanoseconds are actually 24000 hours.
Codecov Report
@@ Coverage Diff @@
## main #1723 +/- ##
===========================================
- Coverage 95.03% 72.89% -22.14%
===========================================
Files 19 18 -1
Lines 10785 4840 -5945
Branches 1725 1058 -667
===========================================
- Hits 10249 3528 -6721
- Misses 523 1030 +507
- Partials 13 282 +269
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
It should be noted that the polyfill implements this differently and is not affected by this change AFAICT. proposal-temporal/polyfill/lib/ecmascript.mjs Lines 1412 to 1420 in 15a1561
proposal-temporal/polyfill/lib/ecmascript.mjs Lines 2858 to 2881 in 15a1561
|
This is also mentioned in #1502. |
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.
Thanks. I don't think this needs extra tests as it is already sufficiently covered that anyone implementing it as written would fail the tests already.
I will mark this as draft so that it's not accidentally merged before being presented for consensus.
This change achieved consensus in TC39. |
The note (and the algorithm itself) claims:
This is incorrect, 24 hours are 8.64 × 10^13 nanoseconds, which is the
value that was intended to be used here (it's already correctly used in
other places). 8.64 × 10^16 nanoseconds are actually 24000 hours.