-
-
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
Refactor Temporal Calendar API for AnyCalendar
and fields
#3522
Conversation
Test262 conformance changes
|
#[boa_gc(unsafe_empty_trace)] | ||
pub struct Calendar { | ||
slot: CalendarSlot, | ||
slot: CalendarSlot<JsCustomCalendar>, |
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 to the new design, we can remove the unsafe_empty_trace
and properly implement Trace
for CalendarSlot<C: Trace>
.
3d11bd4
to
ad30241
Compare
ad30241
to
75f5183
Compare
Marking as ready for review as I believe the core changes for |
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.
Great work! :)
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.
Looks good to me. I agree with @HalidOdat's comment but that should not block the merge :)
This PR is related to the ongoing work for
Temporal
.The largest change on this PR is primarily updating
CalendarSlot
.It changes the following:
CalendarSlot::Protocol(Box<dyn CalendarProtocol>)
toCalendarSlot::Protocol(C)
.CalendarSlot::Identifier(String)
toCalendarSlot::Builtin(AnyCalendar)
CalendarProtocol
to includefields
andmergeFields
.I think there are still a couple things I want to double check/work through. But posting this as a draft in the meantime for any early feedback/thoughts.