-
Notifications
You must be signed in to change notification settings - Fork 168
Use KeySource instead of tuple #399
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 KeySource instead of tuple #399
Conversation
Clippy emits: warning: very complex type used. Consider factoring parts into `type` definitions The type we return is more complex than needed because we do not use the `bip32::KeySource` alias but instead write the full tuple `(byp32::Fingerprint, bip32::DerivationPath)`. To fix the clippy warning we have at least 3 options 1. Add an alias for the complex return type 2. Add a compiler directive to quieten clippy 3. Use `bip32::KeySource` instead of tuple (1) seems like an overkill (2) means we loose lint coverage for the whole function Elect to do (3), the best of 3 imperfect solutions.
ae1eff5 to
f8945d0
Compare
apoelstra
left a comment
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 f8945d0
I actually think this is a great improvement.
sanket1729
left a comment
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 f8945d0
Oh yeah? Why is that? At some other place in some other time I favoured the tuple also because I thought it was clearer than |
|
"keysource" is a standard term (I believe) for this tuple, so by using the shorter name it aids readability. |
|
cool, cheers. |
f8945d05d82b90456fc68812f53807b690a5ab1e Use KeySource instead of tuple (Tobin C. Harding)
Pull request description:
Clippy emits:
warning: very complex type used. Consider factoring parts into `type` definitions
The type we return is more complex than needed because we do not use the `bip32::KeySource` alias but instead write the full tuple `(byp32::Fingerprint, bip32::DerivationPath)`.
To fix the clippy warning we have at least 3 options
1. Add an alias for the complex return type
2. Add a compiler directive to quieten clippy
3. Use `bip32::KeySource` instead of tuple
(1) seems like an overkill
(2) means we loose lint coverage for the whole function
Elect to do (3), the best of 3 imperfect solutions.
ACKs for top commit:
apoelstra:
ACK f8945d05d82b90456fc68812f53807b690a5ab1e
sanket1729:
ACK f8945d05d82b90456fc68812f53807b690a5ab1e
Tree-SHA512: a3f8112d29cdb81d09492277958cad895bdcc4cb933d68b4d25c02cf0ca079922a0d903c3e48ec701bc51f15d8c66d05e9dc7ba56031c28d4a88bc7df1fbaa89
Clippy emits:
warning: very complex type used. Consider factoring parts into
typedefinitionsThe type we return is more complex than needed because we do not use the
bip32::KeySourcealias but instead write the full tuple(byp32::Fingerprint, bip32::DerivationPath).To fix the clippy warning we have at least 3 options
bip32::KeySourceinstead of tuple(1) seems like an overkill
(2) means we loose lint coverage for the whole function
Elect to do (3), the best of 3 imperfect solutions.