Add support for core::ops::Bound #655
Conversation
Codecov Report
@@ Coverage Diff @@
## master #655 +/- ##
==========================================
+ Coverage 64.85% 65.03% +0.17%
==========================================
Files 36 36
Lines 2177 2188 +11
==========================================
+ Hits 1412 1423 +11
Misses 765 765
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
serde_with/src/ser/impls.rs
Outdated
| Bound::Excluded(ref v) => Bound::Excluded(SerializeAsWrap::<T, U>::new(v)), | ||
| } | ||
| .serialize(serializer) | ||
| /* |
There was a problem hiding this comment.
Can you please remove the commented code
There was a problem hiding this comment.
Sorry about that! Done.
serde_with/tests/serde_as/lib.rs
Outdated
| fn test_bound() { | ||
| #[serde_as] | ||
| #[derive(Debug, Serialize, Deserialize, PartialEq)] | ||
| struct S(#[serde_as(as = "_")] Bound<u32>); |
There was a problem hiding this comment.
This does not test your code, but instead the already existing Serialize and Deserialize implementations for Bound. You want as = "Bound<_>".
Can you please add a test that converts the inner type somehow? For example Bound<DisplayFromStr>.
There was a problem hiding this comment.
This does not test your code, but instead the already existing Serialize and Deserialize implementations for Bound. You want
as = "Bound<_>".
Oh, that makes sense! Done.
By the way, wouldn't it be the same with test_option then?
Can you please add a test that converts the inner type somehow? For example
Bound<DisplayFromStr>.
Done.
There was a problem hiding this comment.
Yes, you are right. That test looks wrong too.
|
Thanks for the PR. It makes sense to support Bounds. The code needs some updates, especially the tests. The trait implementations look OK. |
|
Thanks for taking the time to review my changes. I have applied your suggestions. |
|
Thanks for the changes. It looks good now. |
|
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. If you want to switch to GitHub's built-in merge queue, visit their help page. |
I have encountered a case where I would like to use
Bound<Cow<'a, str>>and borrow the string from the deserialized buffer instead of copying. To useBorrowStr, I had to add support forcore::ops::Boundtoserde_with.