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

Support i128 and u128 #219

Merged
merged 1 commit into from
May 1, 2020
Merged

Support i128 and u128 #219

merged 1 commit into from
May 1, 2020

Conversation

e00E
Copy link
Contributor

@e00E e00E commented Apr 30, 2020

This PR adds support for 128 bit integers.

RON does not support 128 bit integers. This is undocumented. The Readme wrongly claims that the full Serde data model is supported.

@e00E
Copy link
Contributor Author

e00E commented Apr 30, 2020

We might want to use https://docs.serde.rs/serde/macro.serde_if_integer128.html. Thoughts?

Data formats that wish to support Rust compiler versions older than 1.26 (or targets that lack 128-bit integers) may place the i128 / u128 methods of their Serializer and Deserializer behind this macro.

Data formats that require a minimum Rust compiler version of at least 1.26, or do not target platforms that lack 128-bit integers, do not need to bother with this macro and may assume support for 128-bit integers.

It is a good idea to be more supportive when this is cheap so I am in favor of including it.

@bh2smith
Copy link

bh2smith commented Apr 30, 2020

I just want to say that this would be a very good addition to the overall ecosystem and that it will become immediately valuable within our project;

gnosis/dex-services#822

Without it, we fear that we may have to implement our own serializations 🤮 !

Really nice clean fix by the way! Looks like these changes fit very naturally in here.

@bh2smith
Copy link

It appears these is another place that needs u128 support? I'm still getting an error when calling ron::de::from_bytes. Have not yet found the culprit.

Copy link
Collaborator

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Amazing work, thank you!
bors r+

bors bot added a commit that referenced this pull request May 1, 2020
219: Support i128 and u128 r=kvark a=e00E

This PR adds support for 128 bit integers.

RON does not support 128 bit integers. This is undocumented. The Readme wrongly claims that the full Serde data model is supported.

Co-authored-by: Valentin Kettner <[email protected]>
@kvark kvark merged commit 7275785 into ron-rs:master May 1, 2020
@kvark
Copy link
Collaborator

kvark commented May 1, 2020

@bh2smith are you able to use the git master for now? Or do you need a release soon?

@bors
Copy link
Contributor

bors bot commented May 1, 2020

Timed out.

@CAD97 CAD97 mentioned this pull request May 1, 2020
@kvark kvark mentioned this pull request May 21, 2020
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