-
Notifications
You must be signed in to change notification settings - Fork 39
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
refactor: change the inner type of Hex
#1371
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
CI tests run on commit:
CI test list:
Please check ci test results later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the current method of handling hex strings.
Also, I have to point that:
-
The serde deserialization is changed.
-
Previous is not strict with
0x
prefix. -
Current is strict with
0x
prefix; no-prefix or0X
prefix is not acceptable.
-
-
But the
from_string
anddecode
is still not strict with0x
prefix.
There should be only one rule for hex conversion to reduce ambiguity.
p.s. I prefer to use the strict method, but I don't persist in.
/run-ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it's a bit more idiomatic to use parse
instead of from_str
directly.
CI tests run on commit:
CI test list:
Please check ci test results later. |
/run-ci |
CI tests run on commit:
CI test list:
Please check ci test results later. |
What this PR does / why we need it?
Change the inner type of
Hex
toBytes
with the following advantages:bytes::Bytes
is very cheap.What is the impact of this PR?
No Breaking Change
PR relation:
CI Settings
CI Usage
Tip: Check the CI you want to run below, and then comment
/run-ci
.CI Switch
CI Description
cargo clippy --all --all-targets --all-features
cargo +nightly fmt --all -- --check
andcargo sort -gwc