time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.06k stars 261 forks source link

Add direct constructors for `OffsetDateTime`. #641

Closed jcdyer closed 6 months ago

jcdyer commented 6 months ago

Fixes #640

This PR adds new constructors to OffsetDateTime:

Rationale for the PR can be found on the linked issue.

jcdyer commented 6 months ago

Can someone help me interpret the output of the coverage build, and understand what actions I should take? To my eye, it looks like a failed snapshot in a part of the codebase I didn't touch. Let me know if I'm missing something, and how I can resolve it.

jcdyer commented 6 months ago

I tend to prefer new over new_in_offset, as I think it is clear enough from the required arguments that it includes the offset, and this is the most natural constructor for an OffsetDateTime. But that's a minor quibble, and I'm happy to do it the way you think makes the most sense.

Changes made as requested, tests added, and ready for a new review.

jhpratt commented 6 months ago

I just realized that this is only implemented in offset_date_time.rs and not in date_time.rs. Given that I'm reworking some of the internals in the latter at the moment, I'll handle that.

This PR looks good. Merging!