Skip to content

Truncate milliseconds to i64 before serializing. #722

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

Merged
merged 1 commit into from
Mar 4, 2025

Conversation

JohnDowson
Copy link
Contributor

A fix for #721

@JohnDowson JohnDowson force-pushed the fix-millisecond-serde branch 2 times, most recently from f8e4920 to 4954bfb Compare January 13, 2025 09:25
@JohnDowson JohnDowson marked this pull request as ready for review January 13, 2025 09:25
@jhpratt jhpratt force-pushed the main branch 2 times, most recently from c866c3c to afa7041 Compare January 20, 2025 05:05
@jhpratt jhpratt force-pushed the fix-millisecond-serde branch from 4954bfb to a5b7dc0 Compare March 4, 2025 08:17
Copy link

codecov bot commented Mar 4, 2025

Codecov Report

Attention: Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.

Project coverage is 93.1%. Comparing base (ce03bca) to head (c0cf53b).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
time/src/serde/timestamp/milliseconds_i64.rs 0.0% 30 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main    #722     +/-   ##
=======================================
- Coverage   93.4%   93.1%   -0.3%     
=======================================
  Files         89      90      +1     
  Lines      10130   10160     +30     
=======================================
- Hits        9462    9460      -2     
- Misses       668     700     +32     

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

@jhpratt jhpratt force-pushed the fix-millisecond-serde branch from a5b7dc0 to c0cf53b Compare March 4, 2025 08:22
@jhpratt jhpratt merged commit f8ecd81 into time-rs:main Mar 4, 2025
21 of 22 checks passed
@JohnDowson
Copy link
Contributor Author

JohnDowson commented Mar 4, 2025

@jhpratt thank you for picking this up!

Sorry I've been unable to find time to finish this up myself.

@jhpratt
Copy link
Member

jhpratt commented Mar 4, 2025

Nah, this was on me. I did a rebase to get CI passing, then after a quick review I saw that deserialization was using i128. Changing that and using the relevant methods from num-conv is all that it took for me to be happy.

As simple of a PR as this is, thank you!

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.

2 participants