-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace time crate with jiff #15293
base: master
Are you sure you want to change the base?
Replace time crate with jiff #15293
Conversation
r? @epage |
@@ -619,7 +620,7 @@ fn auth_token_optional( | |||
if let Some(cached_token) = cache.get(url) { | |||
if cached_token | |||
.expiration | |||
.map(|exp| OffsetDateTime::now_utc() + Duration::minutes(1) < exp) | |||
.map(|exp| Timestamp::now() + Duration::from_secs(60) < exp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small thing, but you could do Timestamp::now() + SignedDuration::from_mins(1)
here.
@@ -367,7 +367,6 @@ dependencies = [ | |||
"tar", | |||
"tempfile", | |||
"thiserror 2.0.11", | |||
"time", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately it looks like this PR doesn't actually remove time
from the dependency tree. As far as I can tell, it's still being brought in by pasetors
.
It looks like it's primarily used by Cargo, so maybe they'd be open to a switch as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for highlighting this.
To be clear, this wouldn't be a blocker for this PR
PR #15290 replaced
humantime
crate withjiff
and this PR replacestime
crate in favor ofjiff
.This should not have functional change.