Conversation
Co-authored-by: sourcefrog <346355+sourcefrog@users.noreply.github.com>
Co-authored-by: sourcefrog <346355+sourcefrog@users.noreply.github.com>
Co-authored-by: sourcefrog <346355+sourcefrog@users.noreply.github.com>
| } | ||
|
|
||
| #[allow(unused)] // really unused at present, but might be useful | ||
| pub(crate) trait ToOffsetDateTime { |
There was a problem hiding this comment.
@copilot maybe just delete this now, unless it's actually used?
There was a problem hiding this comment.
@copilot actually please see if more of this file can be reduced or deleted?
There was a problem hiding this comment.
Simplified in commit 26345ae. Removed all three traits (ToTimestamp, ToFileTime, FromUnixAndNanos) and replaced with:
- Simple helper functions for the two needed conversions
- jiff's built-in
Timestamp::try_from(SystemTime)for SystemTime conversions
The file went from 79 lines to 18 lines.
… built-in conversions Co-authored-by: sourcefrog <346355+sourcefrog@users.noreply.github.com>
Co-authored-by: sourcefrog <346355+sourcefrog@users.noreply.github.com>
Co-authored-by: sourcefrog <346355+sourcefrog@users.noreply.github.com>
AGENTS.md will be in a separate PR focused on contributor guidelines. This PR should focus only on the jiff migration and code formatting.
|
@copilot resolve the merge conflicts in this pull request |
…pilot/convert-time-library-to-jiff Co-authored-by: sourcefrog <346355+sourcefrog@users.noreply.github.com>
- Updated restore.rs to use .to_file_time() method instead of function - Added std::time::Duration import for tests in transport/local.rs - Fixed unused imports in source.rs, transport/local.rs, and transport/s3.rs Co-authored-by: sourcefrog <346355+sourcefrog@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates the codebase’s time handling from the time crate to jiff, updating timestamp/duration operations and adjusting dependencies to align with the new time API.
Changes:
- Update backup duration calculations to use
jiff::Timestamp/jiff::Spaninstead of subtraction ontimetypes. - Adjust tests to use
jiffspan calculations for elapsed time assertions. - Remove the
timefeature flag fromtracing-subscriberconfiguration inCargo.toml.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/show.rs |
Switches displayed backup-duration computation to jiff spans and converts to std::time::Duration for formatting. |
src/band.rs |
Updates a timing-related test to compute elapsed time via jiff::Timestamp::since. |
Cargo.toml |
Removes tracing-subscriber’s explicit time feature from the feature list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let span = end_time.since(info.start_time).unwrap(); | ||
| // Convert jiff::Span to std::time::Duration | ||
| match span.total(jiff::Unit::Nanosecond) { | ||
| Ok(total_nanos) if total_nanos >= 0.0 => { | ||
| let duration = std::time::Duration::from_nanos(total_nanos as u64); | ||
| duration_to_hms(duration).into() | ||
| } | ||
| _ => Cow::Borrowed("negative"), |
There was a problem hiding this comment.
end_time.since(info.start_time).unwrap() can panic if the span computation fails (e.g., corrupted metadata, out-of-range timestamps, or any error case since reports). Since this is user-facing output, handle the Err case and fall back to something like "unknown"/"negative" instead of panicking.
| let span = end_time.since(info.start_time).unwrap(); | |
| // Convert jiff::Span to std::time::Duration | |
| match span.total(jiff::Unit::Nanosecond) { | |
| Ok(total_nanos) if total_nanos >= 0.0 => { | |
| let duration = std::time::Duration::from_nanos(total_nanos as u64); | |
| duration_to_hms(duration).into() | |
| } | |
| _ => Cow::Borrowed("negative"), | |
| match end_time.since(info.start_time) { | |
| Ok(span) => { | |
| // Convert jiff::Span to std::time::Duration | |
| match span.total(jiff::Unit::Nanosecond) { | |
| Ok(total_nanos) if total_nanos >= 0.0 => { | |
| let duration = | |
| std::time::Duration::from_nanos(total_nanos as u64); | |
| duration_to_hms(duration).into() | |
| } | |
| _ => Cow::Borrowed("negative"), | |
| } | |
| } | |
| Err(_) => Cow::Borrowed("unknown"), |
| // Convert jiff::Span to std::time::Duration | ||
| match span.total(jiff::Unit::Nanosecond) { | ||
| Ok(total_nanos) if total_nanos >= 0.0 => { | ||
| let duration = std::time::Duration::from_nanos(total_nanos as u64); | ||
| duration_to_hms(duration).into() | ||
| } | ||
| _ => Cow::Borrowed("negative"), |
There was a problem hiding this comment.
Converting Span to Duration via span.total(Unit::Nanosecond) (a float) and as u64 is lossy (truncation) and can silently saturate on out-of-range values. Prefer an integer-based conversion (or a TryFrom/helper provided by jiff for converting to std::time::Duration) and explicitly handle negative/out-of-range spans instead of relying on float casts.
| // Convert jiff::Span to std::time::Duration | |
| match span.total(jiff::Unit::Nanosecond) { | |
| Ok(total_nanos) if total_nanos >= 0.0 => { | |
| let duration = std::time::Duration::from_nanos(total_nanos as u64); | |
| duration_to_hms(duration).into() | |
| } | |
| _ => Cow::Borrowed("negative"), | |
| match std::time::Duration::try_from(span) { | |
| Ok(duration) => duration_to_hms(duration).into(), | |
| Err(_) => Cow::Borrowed("negative"), |
|
Done elsewhere |
Replaces
timecrate (v0.3) withjiff(v0.2.19) for cleaner time handling APIs and better timezone semantics.Key API Changes
Types:
OffsetDateTime→Timestamp(always UTC, no offset confusion)UtcOffset→TimeZoneMethods:
.unix_timestamp()→.as_second().nanosecond()→.subsec_nanosecond()OffsetDateTime::now_utc()→Timestamp::now()Benefits
TimeZone::system()vsUtcOffset::current_local_offset()Files Modified
src/unix_time.rs(simplified from 79 to 11 lines)src/{band,entry,change}.rs,src/index/entry.rs,src/source/entry.rssrc/transport/{local,sftp,s3}.rssrc/{show,bin/conserve,restore}.rstests/{old_archives,s3_integration}.rsAll modified files formatted with
cargo fmt(import ordering, line breaks).Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.