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

Properly handle i64::MIN when converting to Duration #633

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

hollmmax
Copy link
Contributor

@hollmmax hollmmax commented Aug 3, 2023

fixes #632

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #633 (7503651) into master (1f465b9) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #633      +/-   ##
==========================================
- Coverage   64.47%   64.44%   -0.04%     
==========================================
  Files          36       36              
  Lines        2159     2157       -2     
==========================================
- Hits         1392     1390       -2     
  Misses        767      767              
Files Changed Coverage Δ
serde_with/src/utils/duration.rs 87.34% <100.00%> (-0.16%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jonasbb
Copy link
Owner

jonasbb commented Aug 3, 2023

Thank you for resolving the issue. Indeed, the code in this crate shouldn't panic, but for most cases return a serialization error if something happens. The solution with abs_diff is nice.

I can release the fix tomorrow.

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 16762a7 into jonasbb:master Aug 3, 2023
22 checks passed
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.

TimestampSeconds<i64> panics on i64::MIN
2 participants