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

feat(serde): deprecate individual num::* for a generic quantity module #855

Merged
merged 3 commits into from
Jun 9, 2024

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Jun 8, 2024

Highlights that this is Ethereum JSON-RPC quantity encoding, removes ruint from the name, and makes it generic

Unfortunately #[deprecated] does not emit any warnings for serde(with = "...") invocations

@DaniPopes DaniPopes force-pushed the dani/generic-serde-quantity branch from 33ef3c4 to f41d218 Compare June 8, 2024 19:35
@prestwich
Copy link
Member

I don't mind the name quantity but it should be documented that this is only quantity encoding when in Json (or another human-readable format). For non-human readable formats, it still produces fixed-width binary

@DaniPopes DaniPopes force-pushed the dani/generic-serde-quantity branch from f41d218 to 326e9ff Compare June 8, 2024 19:36
@DaniPopes DaniPopes force-pushed the dani/generic-serde-quantity branch from 75d64d3 to 313f378 Compare June 8, 2024 19:43
crates/serde/src/quantity.rs Outdated Show resolved Hide resolved
Co-authored-by: evalir <[email protected]>
@prestwich prestwich merged commit a81f9e1 into main Jun 9, 2024
24 checks passed
@prestwich prestwich deleted the dani/generic-serde-quantity branch June 9, 2024 21:28
ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
…ule (alloy-rs#855)

* feat(serde): deprecate individual num::* for a generic `quantity` module

* docs: document byte format

* Update quantity.rs

Co-authored-by: evalir <[email protected]>

---------

Co-authored-by: evalir <[email protected]>
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.

4 participants