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

Serialization refactor #247

Closed
greg-szabo opened this issue May 2, 2020 · 2 comments
Closed

Serialization refactor #247

greg-szabo opened this issue May 2, 2020 · 2 comments
Assignees

Comments

@greg-szabo
Copy link
Member

The serializers library started to become a bit unruly:

  • The naming convention is not standardized
  • Deserializer functions for Option<T> versions of different types were created when not necessary

The serializers library should focus on implementing custom ser/de for primitives and complex types that are not implemented by the source code. Things like i64 to String or time::Duration`` to a String that holds the duration in nanoseconds. These should be implemented as functions that can be used in annotation (with=ordeserialize_with=`.)

Custom implementation for custom types should reside with the type and not in the serializers library. (For example CommitSig.)

Together with putting everything in its place, the custom types that need deserialization should deserialize into a type that's easy to use for the developer. The Go types are not necessarily the best mirrored in Rust. Find the Rust-native implementations and make sure that custom serialization can make it compatible with the specification. (Try to separate the issue of serialization from the daily use of the type.)

@greg-szabo
Copy link
Member Author

I'm breaking up PR #248 into multiple, easier-to-handle PRs.

@liamsi
Copy link
Member

liamsi commented Jun 4, 2020

@greg-szabo I think all issues mentioned in the opening comment are addressed for now? And there is also a high-level serialization issue about the different kinds of serialization and overall structure #197. Feel free to re-open if I'm missing sth here.

@liamsi liamsi closed this as completed Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants