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
Replace Vec::from_hex
with hex!
#1438
Conversation
103f434
to
22ab8d7
Compare
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.
ACK 22ab8d7. Thanks for exporting the pub macro in tests. I have versions of this in many downstream repos.
@sanket1729 oh crap, that is a remnant of my failed experiment. It doesn't work the way I intended. I want to change it to Edit: I removed the reexport because it's broken anyway. |
22ab8d7
to
f6bd3a4
Compare
f6bd3a4
to
6faf391
Compare
6faf391
to
f797947
Compare
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.
ACK f797947
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.
ACK f797947
Needs rebase after #1387 |
This makes the code less noisy and is a preparation for changing it to `const`-based literal. Because of the preparation, places that used variables to store the hex string were changed to constants. There are still some instances of `Vec::from_hex` left - where they won't be changeable to `const` and where `hex!` is unavailable (integration tests). These may be dealt with later. See also rust-bitcoin#1189
f797947
to
089a1e4
Compare
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.
ACK 089a1e4
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.
ACK 089a1e4
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.
code review ACk 089a1e4
This makes the code less noisy and is a preparation for changing it to
const
-based literal. Because of the preparation, places that used variables to store the hex string were changed to constants.There are still some instances of
Vec::from_hex
left - where they won't be changeable toconst
and wherehex!
is unavailable (integration tests). These may be dealt with later.See also #1189
Note that while the change appears big it's nearly entirely mechanical, so should be pretty easy to review. (But I don't feel it's
trivial
.)