Run rustfmt on chain/transaction, chain/chaininterface, max_payment_path_len_tests, and chanmon_update_fail_tests
#3783
Conversation
`assert_eq` is generally much better than `assert` for equality tests as it provides debug output of `a` and `b`.
|
👋 Thanks for assigning @joostjager as a reviewer! |
| .unwrap_err(); | ||
| assert_eq!(err, RetryableSendFailure::RouteNotFound); | ||
|
|
||
| // If our payment_metadata contains 1 additional byte, we'll fail prior to pathfinding. | ||
| let mut recipient_onion_too_large_md = recipient_onion_max_md_size.clone(); | ||
| recipient_onion_too_large_md.payment_metadata.as_mut().map(|mut md| md.push(42)); | ||
| let err = nodes[0] |
There was a problem hiding this comment.
For these formatting PRs, I think it is better to keep the formatting strictly separate from any other change. So that reviewers know what to look for in a commit.
This send_payment call is moved down, but as a reviewer I question myself whether it is just formatting or also something that changed functionally. I see it is now called with payment_hash_2 rather than payment_hash.
| route_params.clone(), | ||
| Retry::Attempts(0), | ||
| ) | ||
| .send_payment(payment_hash, max_sized_onion.clone(), id, route_params.clone(), no_retry) |
There was a problem hiding this comment.
My opinion remains unchanged that this isn't necessary to reformat.
| @@ -48,6 +48,9 @@ fn test_monitor_and_persister_update_fail() { | |||
| let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); | |||
| let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); | |||
|
|
|||
| let node_a_id = nodes[0].node.get_our_node_id(); | |||
There was a problem hiding this comment.
This commit, I think the change isn't worth it. A lot of manual checking for a modest reduction of lines of code (less than 10% on this file) and with changes that not everyone may agree benefit readability.
I've only spot-checked this commit. It looks fine, but we need to be aware that it carries some risk. If I had to produce a change like this I'd probably use search/replace/regex and then adding vars where the compiler complains. This may be easier than reviewing all of the 650 lines in this commit that have near-identical changes.
Not that reviewing needs to be easier than authoring necessarily, but it does raise a flag for me whether we should do it at all.
| @@ -38,6 +38,13 @@ use crate::prelude::*; | |||
| use crate::sync::{Arc, Mutex}; | |||
| use bitcoin::hashes::Hash; | |||
|
|
|||
| fn get_latest_mon_update_id<'a, 'b, 'c>( | |||
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
This formats ~36% of the remaining lines in
*/_tests.rs