Skip to content

Conversation

@nekevss
Copy link
Member

@nekevss nekevss commented Sep 11, 2025

This PR is a general audit of the ZonedDateTime built-in.

It completes a variety of changes that would be considered breaking in order to prep for 0.1.

  • Update module and variable name to a full snake case (tz/timezone -> time_zone, zoneddatetime -> zoned_date_time).
    • I did leave some "zdt" in examples for some brevity.
  • Make with_calendar infallible
  • Make a variety of calendar and date/time methods infallible.

@nekevss nekevss requested a review from Manishearth September 11, 2025 01:25
zdt.to_plain_date()
.map(|x| Box::new(Self(x)))
.map_err(Into::into)
Ok(Box::new(Self(zdt.to_plain_date())))
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: change the return type here and below while you're at it, unless you think we want to retain the ability to have this be an error.

Upgrading v8 across this is mildly annoying but fine, especially when there's time to prepare for it

What I don't want is breakages either happening right before a release (takes a few days to prep for) or there being so many of them accumulated between releases that V8 has to carry an unweildy number of ifdefs.

I think we're fine on both counts, here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I left the return type a Result here do to the zdt_from_epoch_ms_with_provider that's called above.

Admittedly, I didn't majorly dive down that codepath here. Primarily because if we are calling anything that requires a provider, then it's going to retain it's Result return unless we remove the use of the provider.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, misread this.

@nekevss nekevss merged commit f888b02 into main Sep 11, 2025
8 checks passed
@nekevss nekevss deleted the api-cleanup-1 branch September 11, 2025 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants