Skip to content
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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

oherrala
Copy link
Contributor

PR #15290 replaced humantime crate with jiff and this PR replaces time crate in favor of jiff.

This should not have functional change.

@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-configuration Area: cargo config files and env vars A-credential-provider Area: credential provider for storing and retreiving credentials A-registry-authentication Area: registry authentication and authorization (authn authz) A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 10, 2025
@oherrala
Copy link
Contributor Author

r? @epage

@rustbot rustbot assigned epage and unassigned ehuss Mar 10, 2025
@@ -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)
Copy link
Member

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",
Copy link
Member

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.

Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars A-credential-provider Area: credential provider for storing and retreiving credentials A-registry-authentication Area: registry authentication and authorization (authn authz) A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants