-
Notifications
You must be signed in to change notification settings - Fork 83
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
deer
: implement Deserialize
for core::num
#2378
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2378 +/- ##
=======================================
Coverage 56.77% 56.78%
=======================================
Files 337 337
Lines 26015 26021 +6
Branches 421 421
=======================================
+ Hits 14769 14775 +6
Misses 11241 11241
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Looks good, but a few more test cases would be nice. I guess this won't be hard 🙂
🌟 What is the purpose of this PR?
Implements Deserialize for:
core::num::Wrapping
core::num::Saturating
Orginally taken from #1875
🔗 Related links
deer
type implementation #1700deer
implementDeserialize
for built-in types (Part 2) #1875👂 Open Questions
Should
Wrapping
andSaturating
be considered at deserialization time? Currently we just deserialize the value with the underlying value and then mapWrapping
onto it. We will still fail if we supply256
to an u8. This is currently whatserde
does.Alternative would be only to implement
Wrapping
on all primitive num types, and if a larger number is encountered we simply modulo it on wrapping or set it to the max.🔍 Has this modified a publishable library?
This PR: