buffa-types: add jiff feature (Timestamp/Duration conversions)#264
Conversation
Add a `jiff` Cargo feature to buffa-types providing conversions between the protobuf well-known types and the jiff crate, mirroring the existing `chrono` feature: - Timestamp <-> jiff::Timestamp (reuses TimestampError; jiff's sign-consistent subsec nanos are re-normalized to proto's non-negative nanos convention by borrowing a second pre-epoch). - Duration <-> jiff::SignedDuration (new DurationJiffError with only InvalidNanos: SignedDuration's range is wide enough that a validated proto Duration never overflows it). no_std-compatible (jiff pulled with default-features=false + alloc); the std feature layers jiff/std on top. Wired into buffa-test for CI coverage. jiff 0.2 MSRV is 1.70, within buffa's 1.75 pin.
|
All contributors have signed the CLA ✍️ ✅ |
- Drop inline comments that restate the doc/code; keep the load-bearing underflow- and overflow-safety rationale. - Add a no_std jiff check to the check-nostd task (parity with chrono).
…ration note
- timestamp_ext.rs's TimestampError and NANOS_MAX docs enumerated exactly
the std and chrono consumers; the jiff conversion is now a third.
- Add a regression test for the subtlest borrow case: one nanosecond
short of jiff's MIN second round-trips through proto { seconds: MIN,
nanos: 1 }, which jiff::Timestamp::new accepts (only negative nanos at
MIN are out of range).
- Changelog: note for chrono users that the jiff Duration conversion has
no Overflow mode, and what to do about it.
- Keep [workspace.dependencies] alphabetical.
No-Verification-Needed: doc/test-only fixups, no product behavior change
iainmcgin
left a comment
There was a problem hiding this comment.
[claude code] Thanks @macalinao — this is an exemplary feature addition. Review verified the load-bearing claims against the jiff 0.2.29 source directly, and everything holds:
jiff::Timestamp::new/SignedDuration::newpanic-freedom: theFromborrow branch is pure integer arithmetic, and theTryFromguards keep every admitted input out of jiff's error/balance paths — including the subtle ones:(MIN, positive nanos)is accepted by jiff (only negative nanos at MIN are out of range), and(i64::MIN, -999_999_999)takesSignedDuration's no-carry path.- Policy parity: the sign-mismatch rejection matches
duration_chrono.rsexactly and agrees with the JSON/text codec'svalidate_duration; the timestamp side correctly relies on normalization instead, because proto's non-negative-nanos convention makes that a genuinely different case. - The
SignedDuration-not-Spanchoice and the absentOverflowmode are both right and both well-argued in the docs. - Feature wiring (
dep:jiff, weakjiff?/std,alloc-only pulling zero transitive crates in no_std config), docs.rsdoc(cfg)coverage, MSRV comment (jiff declares 1.70), Taskfile no_std check, buffa-test wiring, changelog fragment: all verified.
Since the remaining items were trivial, I've applied them directly to your branch (maintainerCanModify), pushed as 4194601 + 74964af:
- Merged
main(the branch was 13 commits behind; clean merge, full workspace clippy + tests pass on the result). timestamp_ext.rs: theTimestampError/NANOS_MAXdocs enumerated exactly the std + chrono consumers — your PR makes jiff a third, so the "both targets" wording had become false.- Added a regression test for the one edge the suite didn't pin: a near-MIN jiff instant with negative subsec nanos, whose borrow produces proto
{ seconds: MIN, nanos: 1 }— the case where the round-trip depends on jiff accepting positive nanos at MIN. - Changelog fragment: added a migration sentence for chrono users about the (correctly) absent
Overflowmode, so the behavioral difference is discoverable before production. - Moved the
jiffworkspace-dependency entry to its alphabetical slot.
Approving. Two deliberate design points a future reader should know were considered and accepted: the JSON-spec-range failure surfacing at serialization time rather than conversion time (inherent to the infallible From, same as chrono), and InvalidNanos covering the sign-mismatch case (matches the chrono convention; the codec's validate_duration is more granular, but changing that here would diverge from DurationChronoError).
|
[claude code] Merged as 699192f — thank you @macalinao! The jiff conversions are a great addition, and the level of care in the edge-case analysis (sign normalization, the Span-vs-SignedDuration rationale, the no_std wiring) made this one of the smoothest external reviews we've had. |
Summary
Adds a
jiffCargo feature tobuffa-typesproviding conversions between the protobuf well-known types andjiff, mirroring the existingchronofeature:Timestamp↔jiff::Timestamp— reuses the sharedTimestampError(InvalidNanos+Overflow).jiff::Timestampreports sub-second nanos sign-consistently (negative pre-epoch), so theFromimpl re-normalizes to proto's always-non-negativenanosconvention by borrowing a second.Duration↔jiff::SignedDuration—SignedDurationis the faithful analog of protoDuration(sign-consistentseconds+ sub-secondnanos;jiff::Spanis deliberately not a target since its calendar units have no fixed length). Introduces a narrowDurationJiffErrorwith onlyInvalidNanos: a validated protoDurationcan never overflowSignedDuration.Details
no_std-compatible:jiffis pulled withdefault-features = false+alloc;buffa-types'stdfeature layersjiff/stdon top viajiff?/std.buffa-testso the conversions are exercised bycargo test --workspacein CI (parity withchrono).jiff0.2 declares MSRV 1.70, within buffa's 1.75 pin (somsrv-checkstays green).jiff::Timestampextremes, bothSignedDurationextremes,i32::MINnanos, bidirectional sign-mismatch rejection.Checks
cargo test -p buffa-types --features jiff✅cargo clippy -p buffa-types --features jiff✅ (clean)cargo check -p buffa-types --no-default-features --features jiff✅ (no_std)cargo fmt --check✅.changes/unreleased/.