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

Issue #285: Add support for lengths other than 64 bits. #294

Closed
wants to merge 11 commits into from

Conversation

@tkaitchuck
Copy link

tkaitchuck commented Dec 13, 2019

This change adds two new config values string_size and array_size.
Both of which take an enum to allow the lengths of strings and vectors/arrays to be set to:
U8, U16, U32, and U64. Where U64 is the default. (Which is compatible with current behavior.)

In the event that the data being serialized does not fit into the size specified the serialization will fail with ErrorKind::SizeTypeLimit

Fixes #285 .

@tkaitchuck
Copy link
Author

tkaitchuck commented Dec 18, 2019

@dtolnay Does this look like a good approach?

Copy link
Collaborator

dtolnay left a comment

Hi @tkaitchuck, I am not really involved with this project these days and I don't get the impression that there are current maintainers who could dedicate time to reviewing something like this in the context of how it fits into a long term design for bincode.

That said, my first impression is that I would prefer if bincode did not support this. I think there is room for someone to make a different data format library that is more geared toward this sort of customization. I think it's fair for bincode to dictate a simple default format and we shouldn't feel the need to build in all manner of customization.

@tkaitchuck
Copy link
Author

tkaitchuck commented Jan 14, 2020

Moved here: pravega#4

@tkaitchuck tkaitchuck closed this Jan 14, 2020
Tristan1900 added a commit to pravega/bincode2 that referenced this pull request Jan 20, 2020
#4)

Add support for lengths other than 64 bits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.