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

Add from_timestamp_nanos #1357

Merged
merged 5 commits into from
Nov 24, 2023
Merged

Add from_timestamp_nanos #1357

merged 5 commits into from
Nov 24, 2023

Conversation

Ali-Mirghasemi
Copy link
Contributor

Add and implement from_timestamp_nanos and add unit test for it

Known issue

if you input i64::MAX function give you a valid datetime, but for i64::MIN it gives you None
i think it's because time unit is nanoseconds

Thanks for contributing to chrono!

If your feature is semver-compatible, please target the 0.4.x branch;
the main branch will be used for 0.5.0 development going forward.

Please consider adding a test to ensure your bug fix/feature will not break in the future.

Add and implement from_timestamp_nanos and add unit test for it
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c0f418b) 91.61% compared to head (82a80c9) 91.64%.
Report is 2 commits behind head on 0.4.x.

Additional details and impacted files
@@            Coverage Diff             @@
##            0.4.x    #1357      +/-   ##
==========================================
+ Coverage   91.61%   91.64%   +0.03%     
==========================================
  Files          38       38              
  Lines       17489    17553      +64     
==========================================
+ Hits        16023    16087      +64     
  Misses       1466     1466              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

#[inline]
#[must_use]
pub const fn from_timestamp_nanos(nanos: i64) -> Option<NaiveDateTime> {
let secs = nanos.div_euclid(1_000_000_000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably reuse the NANOS_PER_SEC const that we have defined somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NANOS_PER_SEC is in duration.rs mod and is inaccessible for me to use it, the solution is make them public for use

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NANOS_PER_SEC will be pub after #1351

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can make it pub(crate) for this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So which one should i use for more compatibility with new commits?

  • add pub(crate) in duration.rs
    or
  • make them pub

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pub(crate), as said in my previous comment.

@djc djc merged commit e1a9494 into chronotope:0.4.x Nov 24, 2023
33 of 35 checks passed
@djc
Copy link
Member

djc commented Nov 24, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants