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

functions to enable encoding/decoding serde types #422

Merged
merged 3 commits into from
Nov 8, 2021

Conversation

VictorKoenders
Copy link
Contributor

This is an initial attempt at supporting serde in bincode 2.

I did have to make separate functions for all serde-related functionality. This is possibly preventable.

Implementing Serialize was relatively easy.

For Deserialize and DeserializeOwned I ended up making 2 functions, which support BorrowDecoder and Decoder respectfully. These also have 2 separate functions right now, we might be able to merge this into one function.

@VictorKoenders VictorKoenders requested a review from ZoeyR October 25, 2021 12:55
@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2021

Codecov Report

Merging #422 (4f1d5de) into trunk (5cbd7fd) will decrease coverage by 6.41%.
The diff coverage is 20.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##            trunk     #422      +/-   ##
==========================================
- Coverage   71.04%   64.62%   -6.42%     
==========================================
  Files          41       47       +6     
  Lines        2811     3237     +426     
==========================================
+ Hits         1997     2092      +95     
- Misses        814     1145     +331     
Impacted Files Coverage Δ
serde.rs 0.00% <0.00%> (ø)
src/de/decoder.rs 42.85% <ø> (-28.58%) ⬇️
src/de/impls.rs 75.00% <0.00%> (-0.59%) ⬇️
src/enc/encoder.rs 57.14% <ø> (-14.29%) ⬇️
src/enc/impls.rs 89.06% <0.00%> (-1.90%) ⬇️
src/error.rs 0.00% <0.00%> (ø)
src/features/impl_std.rs 71.75% <ø> (ø)
src/features/serde/mod.rs 0.00% <0.00%> (ø)
src/features/serde/ser.rs 20.20% <20.20%> (ø)
src/features/serde/de_borrowed.rs 21.50% <21.50%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cbd7fd...4f1d5de. Read the comment docs.

Copy link
Collaborator

@ZoeyR ZoeyR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should have an impl Encode for T where T: Serialize and same for Decode. With the current implementation you can't bring a struct that only implements Serialize into a struct you want to implement Encode on

@VictorKoenders
Copy link
Contributor Author

I've opened a separate issue for having bincode Decode/Encode work together with serde's traits here: #433

@VictorKoenders VictorKoenders merged commit a2e7e0e into trunk Nov 8, 2021
@VictorKoenders VictorKoenders deleted the bincode2/serde_impl branch November 8, 2021 11:38
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.

3 participants