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

Use parse::<Txid>() in place of encode::deserialize_hex() [issue 11] #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fedemagnani
Copy link

Closes #11

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK f2f891b; successfully ran local tests; interesting about the changed error type

@tcharding
Copy link
Member

tcharding commented Dec 3, 2024

This is awesome; I had not imagined we would get rid of the encode::FromHex error returns - that is cool. Thanks

Can I be a pest and ask you to write a more complete commit log please

Something like

Use parse::<Txid> instead of encode::deserialize_hex

Currently, in some places, we use the bespoke `encode::deserialize_hex` function to deserialize txid from a hex string. The `Txid` type provides a `FromStr` impl so we can use `parse::<Txid>()`. This makes the code more easily readable because its a common pattern in Rust.

(If you cut'n'past don't use a single line for the body, max column width 72 please.)

Thanks man.

If you are interested in git commit logs check out the very educational post: https://cbea.ms/git-commit/

Currently, in some places, we use the bespoke `encode::deserialize_hex`
function to deserialize txid from a hex string. The `Txid` type provides a
`FromStr` impl so we can use `parse::<Txid>()`. This makes the code
more easily readable because its a common pattern in Rust.
@fedemagnani
Copy link
Author

fedemagnani commented Dec 3, 2024

Thank you guys. I amended the commit including the message sugested. Does it look good? @tcharding

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.

Use parse instead of deserialize_hex for hash types
3 participants