-
Notifications
You must be signed in to change notification settings - Fork 260
Fix missing support for 128-bit integers #731
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
base: main
Are you sure you want to change the base?
Conversation
3ca2522 to
f99d86e
Compare
Pull Request Test Coverage Report for Build 21673872770Details
💛 - Coveralls |
f99d86e to
e06fd8c
Compare
e06fd8c to
6981823
Compare
|
CI error with failed coverage publishing is not relevant to this PR. Everything else made green. |
| fn serialize_i128(self, v: i128) -> Result<Self::Ok> { | ||
| if v > i64::MAX.into() { | ||
| Err(ConfigError::Message(format!( | ||
| "value {} is greater than the max {}", | ||
| v, | ||
| i64::MAX | ||
| ))) | ||
| } else if v < i64::MIN.into() { | ||
| Err(ConfigError::Message(format!( | ||
| "value {} is less than the min {}", | ||
| v, | ||
| i64::MIN | ||
| ))) | ||
| } else { | ||
| self.serialize_i64(v as i64) | ||
| } |
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.
Why are we checking the ranges rather than serializing as a primitive?
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.
@epage I don't know. I did repeat the logic that was already there for the serialize_u64(). It seems strange to me too, but I can only speculate that it's there for compatibility with JSON integer types, or something.
Personally, I don't see any value in it. But changing this would be a subtle breaking change, so seemed to be out of scope for this PR.
Complements #178
Related to #161
Synopsis
At the moment trying to deserialize
Configinto a struct withu128/i128errors:This happens even if the parsed value is small and fits into other integers.
Solution
This PR wires the
Value'si128/u128functionality intoDeserializer/Serializerimplementations.