Skip to content

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
@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