bitcoind_test: Fix clippy #686
Conversation
Remove build error: error: casting integer literal to `u8` is unnecessary
| @@ -86,8 +86,8 @@ fn setup_keys( | |||
| let mut x_only_keypairs = vec![]; | |||
| let mut x_only_pks = vec![]; | |||
|
|
|||
| for i in 0..n { | |||
| let keypair = bitcoin::secp256k1::Keypair::from_secret_key(&secp_sign, &sks[i]); | |||
| for sk in sks.iter().take(n) { | |||
There was a problem hiding this comment.
In b328736:
iter and take continue to be unnecessary
There was a problem hiding this comment.
Its like I didn't even read your review the first time :(
There was a problem hiding this comment.
Will fix, and lol "continue to be unnecessary" - love it.
There was a problem hiding this comment.
I don't know what was the original intention of the code but as it reads the take(n) is required because n is a function parameter so we can't just iterate over all of sk.
There was a problem hiding this comment.
In the iteration above we construct sk by looping n times and pushing stuff onto sk.
We could add a debug_assert that sk.len() equals n if it makes you nervous.
There was a problem hiding this comment.
My bad, I did not read the code properly. Used for sk in sks { as suggested.
|
Should we squash all these commits down? |
|
@storopoli I see no particular reason to. They are all independent and compile and pass all tests (I assume -- I have not tested them because the second one needs to be changed). |
|
I go to painful lengths to put all these changes separately so that its quick and easy for reviewers to review them [0]. Also its easy to drop any particular patch if a reviewer doesn't like it. Even the commit messages should be so uniform that they are easy to read quickly (and mechanical for me to write). [0] Definitely pull me up if there are changes in the wrong place. |
Clear clippy warning: the loop variable `i` is only used to index `sks`
Clippy emits: warning: redundant field names in struct initialization As suggested, remove redundant field names.
Clippy emits: warning: single-character string constant used as pattern As suggested, use single quotes.
Clippy emits: warning: manual implementation of an assign operation As suggested, use += operator.
Clippy emits: warning: unneeded `return` statement As suggested, remove unneeded return statement.
Clippy emits: warning: field assignment outside of initializer for an instance created with Default::default() As suggested use `..Default::default()` instead of mutating.
Clippy emits: warning: this expression creates a reference which is immediately dereferenced by the compiler As suggested, remove explicit reference.
Clippy emits: warning: useless conversion to the same type: `setup::miniscript::bitcoin::absolute::LockTime` As suggested, remove useless conversion.
Found with clippy, just use `Copy`.
Clippy emits: warning: use of `expect` followed by a function call As suggested, use `unwrap_or_else` with `panic!`.
Clippy emits: warning: redundant pattern matching, consider using `is_err()` As suggested, use `is_err()`.
|
Neat, TIL:
|
|
Oops, that utACk should have been a full ACK. I did test every commit, it just took some manual prodding to avoid opening too many files at once. |
|
Is there anything we can do about bot accounts triggering notifications like this @ErnestoLara idiot above? (Mentioning him incase he is a real person :) |
All the clippy patches rom #682, only touches
bitcoind_test.