-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
libtime: Use Duration in Timespec arithmetic. #16650
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
Conversation
|
This looks great, thanks! It may take #16626 more time to land than this, so perhaps this could be rebased on master and this could land first? |
7df21fa to
913f0ff
Compare
|
This works with the current |
src/libtime/lib.rs
Outdated
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.
Could you align this .num_nanoseconds() to the right to not be left-aligned?
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.
I wondered about that, what the best style is (I could not find anything in the style guide). Do you mean aligning it with the opening parenthesis?
913f0ff to
cb8389d
Compare
|
Now that #16626 has been approve, this should probably wait to get merged, other than that this looks good to me! |
|
Now that #16626 has merged, should this be rebased to remove the FIXMEs? |
|
Yes, I’m running |
cb8389d to
d20de6d
Compare
This changes the `Add` and `Sub` implementations for `Timespec` introduced in #16573 to use `Duration` as the time span type instead of `Timespec` itself, as [suggested](#16573 (comment)) by @sfackler. This depends on #16626, because is uses `Duration::seconds(i64)`, whereas currently `Duration::seconds` takes an `i32`.
Fix: Fix metrics CI failing
This changes the
AddandSubimplementations forTimespecintroduced in #16573 to useDurationas the time span type instead ofTimespecitself, as suggested by @sfackler.This depends on #16626, because is uses
Duration::seconds(i64), whereas currentlyDuration::secondstakes ani32.