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

Consider adding a "Default" implementation to Duration #717

Closed
eminence opened this issue Jun 22, 2022 · 10 comments
Closed

Consider adding a "Default" implementation to Duration #717

eminence opened this issue Jun 22, 2022 · 10 comments
Labels
API-incompatible Tracking changes that need incompatible API revisions
Milestone

Comments

@eminence
Copy link

Would you consider having chrono::Duration implement Default, which would produce a zero-duration Duration ?

This would be consistent with std::time::Duration

@djc
Copy link
Member

djc commented Jun 23, 2022

Right now the implementation of the Duration type depends on the Cargo features: it can be from time 0.1 or the internal version. The external version is enabled by default, so if we implement this only for the internal version, so this would make the oldtime Cargo feature non-additive. On the other hand... I guess it already is?

@eminence
Copy link
Author

Ah, I had forgotten about oldtime. chrono::Duration already had a slightly different API from time::Duration. For example chrono::Duration implements Hash but time::Duration does not. (Though otherwise their APIs are nearly identical).

Maybe chrono 0.5 could add a impl Default for Duration ?

@djc
Copy link
Member

djc commented Jun 27, 2022

I'd be happy to add that once we straighten out the compatibility situation.

@djc djc added the API-incompatible Tracking changes that need incompatible API revisions label Jun 27, 2022
@esheppa esheppa added this to the 0.5 milestone Aug 2, 2022
@delightfulagony
Copy link

What's the current state of this issue?

As far as I understood from @djc, the reason for not implementing Default was because std::time::Duration didn't implement it in 0.1.x.

As of 0.3.17 default is implemented, does that mean this is no longer "API-Incompatible" and could be implemented?

@djc
Copy link
Member

djc commented Nov 23, 2022

On the main branch (which will eventually become 0.5) we can add a Default implementation to what's now called TimeDelta. On the 0.4.x branch due to compatibility concerns that will not be viable.

You seem to be confusing the std::time::Duration type with the Duration type from the time crate. Due to compatibility, chrono 0.4.x will always (conditionally) depend on time 0.1, while chrono 0.5 does not depend on time at all.

@delightfulagony
Copy link

You're right, I mistook std::time with the time crate.

But if this time I checked right, as of 0.1.3 for std::time::Duration, default is also implemented

@djc
Copy link
Member

djc commented Nov 23, 2022

Yes, but the Duration we expose in chrono is actually the one from the time crate, so std::time::Duration has no bearing on this issue.

@esheppa
Copy link
Collaborator

esheppa commented Nov 23, 2022

@delightfulagony - if this #860 or a linked PR gets merged, then you will be able to use std::time::Duration directly, including its Default implementation, would that satisfy your use case?

@delightfulagony
Copy link

delightfulagony commented Nov 24, 2022

Yes, but the Duration we expose in chrono is actually the one from the time crate, so std::time::Duration has no bearing on this issue.

Oh ok, so I misunderstood you, alright, thank you

@delightfulagony - if this #860 or a linked PR gets merged, then you will be able to use std::time::Duration directly, including its Default implementation, would that satisfy your use case?

I suppose it would, in any case I changed to using the time crate that also works for my needs, and since you folks are working on 5.x.x don't worry about it, thank you for taking the time to answer and explain 😄

You're both the best! 🎉 👍

@pitdicker
Copy link
Collaborator

Added in #1327.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-incompatible Tracking changes that need incompatible API revisions
Projects
None yet
Development

No branches or pull requests

5 participants