Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Throw on negative values when usingOverflow::Reject inTemporal.PlainTime.with#4490

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
HalidOdat merged 3 commits intomainfromfix/temporal.plaintime.with
Oct 27, 2025

Conversation

@HalidOdat
Copy link
Member

This PR fixes the last failing test on theTemporal.PlainTime test suite (throws-if-time-is-invalid-when-overflow-is-reject.js).

@github-actions
Copy link

Test262 conformance changes

Test resultmain countPR countdifference
Total50,59550,5950
Passed47,63547,656+21
Ignored2,0562,036-20
Failed904903-1
Panics000
Conformance94.15%94.19%+0.04%
Fixed tests (21):
test/built-ins/Temporal/PlainTime/prototype/with/throws-if-time-is-invalid-when-overflow-is-reject.js (previously Failed)test/built-ins/RegExp/escape/length.js (previously Ignored)test/built-ins/RegExp/escape/escaped-whitespace.js (previously Ignored)test/built-ins/RegExp/escape/cross-realm.js (previously Ignored)test/built-ins/RegExp/escape/escaped-solidus-character-mixed.js (previously Ignored)test/built-ins/RegExp/escape/escaped-syntax-characters-simple.js (previously Ignored)test/built-ins/RegExp/escape/not-a-constructor.js (previously Ignored)test/built-ins/RegExp/escape/escaped-otherpunctuators.js (previously Ignored)test/built-ins/RegExp/escape/escaped-utf16encodecodepoint.js (previously Ignored)test/built-ins/RegExp/escape/name.js (previously Ignored)test/built-ins/RegExp/escape/not-escaped-underscore.js (previously Ignored)test/built-ins/RegExp/escape/prop-desc.js (previously Ignored)test/built-ins/RegExp/escape/not-escaped.js (previously Ignored)test/built-ins/RegExp/escape/initial-char-escape.js (previously Ignored)test/built-ins/RegExp/escape/non-string-inputs.js (previously Ignored)test/built-ins/RegExp/escape/escaped-surrogates.js (previously Ignored)test/built-ins/RegExp/escape/escaped-control-characters.js (previously Ignored)test/built-ins/RegExp/escape/is-function.js (previously Ignored)test/built-ins/RegExp/escape/escaped-lineterminator.js (previously Ignored)test/built-ins/RegExp/escape/escaped-syntax-characters-mixed.js (previously Ignored)test/built-ins/RegExp/escape/escaped-solidus-character-simple.js (previously Ignored)

@HalidOdatHalidOdat marked this pull request as ready for reviewOctober 24, 2025 22:22
@HalidOdatHalidOdat requested a review froma teamOctober 24, 2025 22:22
@jedel1043
Copy link
Member

Why not upstream this totemporal_rs? Is this behaviour only visible from the engine side?

@HalidOdat
Copy link
MemberAuthor

HalidOdat commentedOct 25, 2025•
edited
Loading

Why not upstream this totemporal_rs? Is this behaviour only visible from the engine side?

The issue is thatPartialTime intemporal_rs expects only unsigned integers, while the specification allows negative integers too. This means the validation can't be delayed after overflow retrieval (hence the two[Js]PartialTime). From a Rust API design standpoint as a standalone library, restrictingPartialTime to the narrowest valid type makes sense (less margin for error from the user).

I’d be interested to hear what@nekevss thinks as well? 😄

@nekevss
Copy link
Member

Seeboa-dev/temporal#432 andboa-dev/temporal#334. So yes, the issue has been known and preferably this behavior should be upstream intemporal_rs. The question has been the best way to represent this intemporal_rs's API.

It is pretty niche behavior, so I've delayed adding it. My goal is to ultimately land something in temporal_rs before 1.0.

HalidOdat reacted with eyes emoji

Copy link
Member

@jedel1043jedel1043 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Makes sense. I don't have any other suggestions then. We can remove this if/when we change the behaviour intemporal_rs.

HalidOdat reacted with thumbs up emoji
@HalidOdatHalidOdat added this pull request to themerge queueOct 27, 2025
Merged via the queue intomain with commit59a7566Oct 27, 2025
15 checks passed
@HalidOdatHalidOdat deleted the fix/temporal.plaintime.with branchOctober 27, 2025 15:04
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jedel1043jedel1043jedel1043 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HalidOdat@jedel1043@nekevss
[8]ページ先頭
©2009-2025 Movatter.jp