-
-
Notifications
You must be signed in to change notification settings - Fork 411
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
Fix a few Duration code typos #3730
Fix a few Duration code typos #3730
Conversation
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.
Nice catch! Thanks for the fix 😄
EDIT: Conformance results
Test262 conformance changes
Test result | main count | PR count | difference |
---|---|---|---|
Total | 50,268 | 50,268 | 0 |
Ignored | 1,391 | 1,391 | 0 |
Panics | 0 | 0 | 0 |
Conformance | 85.05% | 85.08% | +0.03% |
Fixed tests (16):
test/built-ins/Temporal/Instant/prototype/add/subclassing-ignored.js (previously Failed)
test/built-ins/Temporal/Instant/prototype/add/basic.js (previously Failed)
test/built-ins/Temporal/Instant/prototype/subtract/subclassing-ignored.js (previously Failed)
test/built-ins/Temporal/Instant/prototype/subtract/basic.js (previously Failed)
test/built-ins/Temporal/Duration/mixed.js (previously Failed)
test/built-ins/Temporal/Duration/fractional-throws-rangeerror.js (previously Failed)
test/built-ins/Temporal/Duration/basic.js (previously Failed)
test/built-ins/Temporal/Duration/prototype/with/copy-properties-not-undefined.js (previously Failed)
test/built-ins/Temporal/Duration/prototype/with/all-positive.js (previously Failed)
test/built-ins/Temporal/Duration/prototype/with/all-negative.js (previously Failed)
test/built-ins/Temporal/Duration/prototype/with/partial-positive.js (previously Failed)
test/built-ins/Temporal/Duration/prototype/with/subclassing-ignored.js (previously Failed)
test/built-ins/Temporal/Duration/prototype/abs/subclassing-ignored.js (previously Failed)
test/built-ins/Temporal/Duration/prototype/abs/basic.js (previously Failed)
test/built-ins/Temporal/PlainTime/prototype/add/balance-negative-time-units.js (previously Failed)
test/built-ins/Temporal/PlainTime/prototype/add/subclassing-ignored.js (previously Failed)
Broken tests (1):
test/built-ins/Temporal/Duration/prototype/with/sign-conflict-throws-rangeerror.js (previously Passed)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3730 +/- ##
==========================================
+ Coverage 47.24% 47.67% +0.42%
==========================================
Files 476 454 -22
Lines 46892 44563 -2329
==========================================
- Hits 22154 21245 -909
+ Misses 24738 23318 -1420 ☔ View full report in Codecov by Sentry. |
No problem! Good way to get my feet wet. Gonna see if I can increase test passes by more than 12 next time haha |
Sometimes it's only a handful of tests other times it's more! 😄 If you want to keep working through the Temporal builtin and need another place to look, there are tracking issues for the native rust implementation of Temporal here. It may be a good place to find out where there's functionality that hasn't been able to make it into |
This Pull Request fixes conformance test failures identified in
test/built-ins/Temporal/Duration/prototype/with/partial-positive.js
.It fixes some typos found in the Duration impl