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

Make public deserialize methods deser_v2 & deser_v2_compressed #84

Open
EliSnow opened this issue Aug 23, 2019 · 3 comments
Open

Make public deserialize methods deser_v2 & deser_v2_compressed #84

EliSnow opened this issue Aug 23, 2019 · 3 comments

Comments

@EliSnow
Copy link

EliSnow commented Aug 23, 2019

Specifically I'm interested in calling the deser_v2 method directly. My motivation is I'm using hdrhistogram in a WASM module where I only need to deserialize uncompressed histograms. By using the deserialize method the compiler can not statically determine that I will not be deserializing compressed histograms and flate2 gets included in my WASM.

I only included deser_v2_compressed in the title because I figured if you make public one of them then perhaps it makes sense to do both(??).

@jonhoo
Copy link
Collaborator

jonhoo commented Aug 24, 2019

I think @marshallpierce has more insight into what else we might want to do before making these public. Might be fine to just mark them pub, but want to check with him first.

@marshallpierce
Copy link
Collaborator

Interesting. It's a bit of an abstraction-layer mangling to simply mark them pub, but I definitely want to support WASM use cases!

Anyone have thoughts on other ways to do this, perhaps by making a trait to represent derserialization strategies?

@EliSnow
Copy link
Author

EliSnow commented Oct 24, 2019

There are two structs for serializing: V2Serializer and V2DeflateSerializer. Would it make sense to follow this pattern for deserializing? V2Deserializer and V2DeflateDeserializer? The only hickup I see is that the two serializers implement a common trait Serializer and if we wanted the deserializers to implement a common trait, it would have to be named something other than Deserializer since that's taken by the struct.

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

No branches or pull requests

3 participants