-
Notifications
You must be signed in to change notification settings - Fork 831
Replace Vec::from_hex
with hex!
#1438
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
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 22ab8d71a6b1d30794ac9502fd9449423e2ee7f5. 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 other 8000 s. Learn more.
ACK f797947f4dab8e3f81e67e22e338219b62fa5784
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 f797947f4dab8e3f81e67e22e338219b62fa5784
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 insta 8000 nces 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
.)