Replace chrono feature with jiff - #148
Open
fabian-braun wants to merge 16 commits into
Open
fabian-braun wants to merge 16 commits into
fabian-braun wants to merge 16 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the optional chrono-backed integration with a jiff-backed one, migrating RFC3339 parsing/formatting, Display, and conversion impls behind a new jiff feature flag, and updates expectations around infinity formatting.
Changes:
- Replace the optional
chronofeature/dependency withjiff, including deletingsrc/chrono.rsand addingsrc/jiff.rs. - Port
TimeRFC3339 parse/format andDisplay/conversion impls tojiffequivalents, including±∞formatting for out-of-range values. - Update tests and changelog to reflect the feature rename and new formatting expectations.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/serde.rs | Switch RFC3339 serde helper gating from chrono to jiff. |
| src/lib.rs | Rename public feature module to jiff and adjust formatting-related tests. |
| src/jiff.rs | New jiff-based formatting/parsing/conversion implementations and Display impls. |
| src/chrono.rs | Removed chrono-based integration module. |
| CHANGELOG.md | Document the breaking feature rename (chrono → jiff). |
| Cargo.toml | Swap optional dependency/feature from chrono to jiff. |
| Cargo.lock | Update lockfile to remove chrono-related crates and add jiff-related crates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
fabian-braun
marked this pull request as ready for review
June 15, 2026 18:09
Felerius
reviewed
Aug 25, 2026
Felerius
left a comment
Felerius
left a comment
Contributor
There was a problem hiding this comment.
I like the idea of switching! Some nitpicky comments below.
| use crate::Time; | ||
| use crate::TimeWindow; | ||
|
|
||
| const RFC3339_FORMAT: &str = "%Y-%m-%dT%H:%M:%S+00:00"; |
Contributor
There was a problem hiding this comment.
Display of jiff::Timestamp creates an RFC3339 timestamp. Maybe we want to go through that instead of having this custom format string? That also avoids the discussion whether we want +00:00 vs Z.
Contributor
Author
There was a problem hiding this comment.
Done in d213bba — to_rfc3339/Display now go through the Display impl of jiff::Timestamp, so UTC renders as Z. Note this also means subsecond digits show up (2024-02-06T16:53:47.962Z); documented in the changelog.
Co-Authored-By: Claude <noreply@anthropic.com>
Addresses review comment: the display/format tests belong next to the jiff-backed implementations. Co-Authored-By: Claude <noreply@anthropic.com>
Addresses review comment: go through the Display impl of jiff::Timestamp instead of a custom strftime format string. UTC is now rendered as "Z" and subsecond digits are included. Co-Authored-By: Claude <noreply@anthropic.com>
Addresses review comment: the infinity cases can be handled by handing the "∞" / "-∞" literal to strftime as the format string, which removes the need for the FormattedTime wrapper. Co-Authored-By: Claude <noreply@anthropic.com>
Addresses review comment: out-of-range durations can't be represented, so surface that as an error instead of a debug assertion plus saturation. Co-Authored-By: Claude <noreply@anthropic.com>
jiff gates `impl std::error::Error for Error` behind its `std` feature, so without it `jiff::Error` didn't integrate with the usual error handling. Co-Authored-By: Claude <noreply@anthropic.com>
Otherwise the feature-gated items, such as `Time::format()`, are missing. Co-Authored-By: Claude <noreply@anthropic.com>
`Timestamp::from_millisecond()` truncates its upper bound to a whole second, which made the last 999 representable milliseconds render as "∞" and broke the round trip from `Timestamp::MAX`. `Timestamp::from_duration()` doesn't. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
chronofeature/dependency withjiffDisplay, and conversion impls to jiff equivalentsFixes #147