-
Notifications
You must be signed in to change notification settings - Fork 542
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
Rename Duration
to TimeDelta
, add type alias
#1406
Conversation
a1a070a
to
dc814ab
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1406 +/- ##
=======================================
Coverage 91.84% 91.85%
=======================================
Files 38 38
Lines 17518 17526 +8
=======================================
+ Hits 16090 16098 +8
Misses 1428 1428 ☔ View full report in Codecov by Sentry. |
4c84e2d
to
174f04a
Compare
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.
Did a review of all lines. It was not a trivial find-and-replace, but it seems like I didn't mangle things.
You refer to |
|
There was a little discussion on possible names in #1137 (comment). I don't care deeply about the names, but |
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.
Okay -- do we want to deprecate the Duration
alias, and if so, when?
src/lib.rs
Outdated
@@ -467,6 +467,8 @@ extern crate alloc; | |||
|
|||
mod duration; | |||
pub use duration::Duration; | |||
/// Alias of [`Duration`] |
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.
This should be formatted as a type, that is, with empty lines around it and not between a bunch of imports.
src/lib.rs
Outdated
//! [`Duration::from_std`](https://docs.rs/time/0.1.40/time/struct.Duration.html#method.from_std) | ||
//! and | ||
//! [`Duration::to_std`](https://docs.rs/time/0.1.40/time/struct.Duration.html#method.to_std) | ||
//! ### Time span / Duration |
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.
Let's use "Time delta" in the title here?
Thank you! Looking forward to less merge conflicts (going to introduce plenty of new ones though now that I'm starting to work on the 0.5 branch).
I feel that deprecating the type would cause a lot of churn while bringing little benefit. New code can start using |
We have discussed something like this many times. Reasons to rename
Duration
toTimeDelta
:Duration
is usually understood to be strictly positive. Our type can be both positive and negative. That makes it more flexible, which is a good thing for a date and time library. The nameTimeDelta
can better represent a signed value.Duration
type unsigned, as that is a better match for system APIs. We want to avoid confusion with that type.OldDuration
we currently use as an alias in our documentation to ease the confusion a little is just sad (and not really helping).TimeDelta
, and we expect users to eventually migrate. By making the name available now we can ease the migration a little when the time comes.The old name
Duration
will remain available as a type alias forTimeDelta
, so this is not a breaking change.