-
Notifications
You must be signed in to change notification settings - Fork 594
Add subsec_micros and subsec_millis methods to TimeDelta
#1668
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1668 +/- ##
==========================================
+ Coverage 91.05% 91.07% +0.01%
==========================================
Files 37 37
Lines 17402 17431 +29
==========================================
+ Hits 15846 15875 +29
Misses 1556 1556 ☔ View full report in Codecov by Sentry. |
|
Honestly I'm not convinced by the argument that these consts should be made public just so two of them can make a single appearance in a docstring. IMO it would be fine to leave the constants alone and use literals to explain the methods. |
5aee138 to
25936a4
Compare
Makes sense to me -- and there was precedent with the previous docstring for |
| /// Returns the number of nanoseconds such that | ||
| /// Returns the number of nanoseconds in the fractional part of the duration. | ||
| /// | ||
| /// This is the number of nanoseconds such that |
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.
Sorry, by literal I meant to replace NANOS_PER_SEC with 1_000_000_000 here (and same for the new methods). Other than that this seems fine!
(Maybe also update the PR description to describe the current approach.)
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.
Updated the 3 comments and the PR description.
Co-authored-by: Micha White <[email protected]>
Co-authored-by: Micha White <[email protected]>
25936a4 to
0d4b934
Compare
This PR revives @botahamec 's PR #1351, which has been abandoned, and adds
subsec_microsandsubsec_millisto the public API forchrono::TimeDelta, for consistency with the API ofstd::time::Duration.Summary of changes:
TimeDelta::subsec_nanos()and add a test for the method.subsec_millisandsubsec_micros, along with unit tests.Fixes: #1348