-
Notifications
You must be signed in to change notification settings - Fork 827
Refactor use map_err #794
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 use map_err #794
Conversation
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 believe there was a second place in that PR when the similar matching was happening?
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.
@dr-orlovsky yes, there was another place
src/util/psbt/serialize.rs
Outdated
Ok(sig) => Ok(sig), | ||
8000 Err(schnorr::SchnorrSigError::InvalidSighashType(flag)) => { | ||
Err(encode::Error::from(psbt::Error::NonStandardSigHashType(flag as u32))) | ||
schnorr::SchnorrSig::from_slice(&bytes).map_err(|e| match e { |
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 feel that one of these styles would be easier to understand by readers:
schnorr::SchnorrSig::from_slice(&bytes)
.map_err(|e| match e {
...
})
schnorr::SchnorrSig::from_slice(&bytes)
.map_err(|e|
match e {
...
}
)
schnorr::SchnorrSig::from_slice(&bytes)
.map_err(|e|
match e {
...
}
)
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.
thank you for review.
fixed code!
ee4e405
to
9f84847
Compare
i have refactor another place too |
I approved run but unless I'm too confused something it'll fail on missing commas. |
What the hell?! Commas are not mandatory? OMG I thought I knew Rust well. 🤣 |
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 9f84847
Thank you!
According to the reference, comma after a BlockExpression(like https://doc.rust-lang.org/reference/expressions/match-expr.html |
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 9f84847
issue: #793
change to using map_err