Rustfmt tests that touch persistence#3882
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
A few patterns worth cleaning up imo across the diff.
|
|
||
| nodes[0].enable_channel_signer_op(&nodes[1].node.get_our_node_id(), &channel_id_0, SignerOp::GetPerCommitmentPoint); | ||
| nodes[0].enable_channel_signer_op( | ||
| &nodes[1].node.get_our_node_id(), |
There was a problem hiding this comment.
Do you want to go ahead and do the global search+replace for nodes[n].node.get_our_node_id -> node_n_id (and (\t*)(let nodes = create_network.*) -> \1\2\n\1let node_n_id = nodes[n].node.get_our_node_id)?
There was a problem hiding this comment.
Added commit with script in commit message
| nodes[0].node.handle_accept_channel(nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id())); | ||
| nodes[0].node.handle_accept_channel( | ||
| nodes[1].node.get_our_node_id(), | ||
| &get_event_msg!( |
There was a problem hiding this comment.
Can we pull the get_event_msg calls out into variables?
There was a problem hiding this comment.
With the node ids extracted, this wrapping doesn't seem to be happening anymore.
| src.node | ||
| .peer_connected( | ||
| dst.node.get_our_node_id(), | ||
| &msgs::Init { |
There was a problem hiding this comment.
Can you pull the Init message out into a variable (and reuse it in the second connect below)?
There was a problem hiding this comment.
Done. Second one isn't the same.
| check_closed_event( | ||
| &nodes[1], | ||
| 1, | ||
| ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true) }, |
There was a problem hiding this comment.
Can we break this out into a reason? Also this is gonna be annoying to rebase #3881, can we do this after that lands?
There was a problem hiding this comment.
The delta in that PR is tiny. Seems the lowest level of annoying?
There was a problem hiding this comment.
Extracted this reason. Not very necessary in my opinion.
|
👋 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. |
joostjager
force-pushed
the
rustfmt-async-kv-store
branch
3 times, most recently
from
2d084d8 to
2c2415e
Compare
|
🔔 1st Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
Script used:
git reset --hard
for file in lightning/src/ln/async_signer_tests.rs lightning/src/ln/functional_test_utils.rs lightning/src/ln/priv_short_conf_tests.rs lightning/src/ln/shutdown_tests.rs; do
sed -i '' -E \
-e 's/nodes\[0\]\.node.get_our_node_id\(\)/node_a_id/g' \
-e 's/nodes\[1\]\.node.get_our_node_id\(\)/node_b_id/g' \
-e 's/nodes\[2\]\.node.get_our_node_id\(\)/node_c_id/g' \
-e 's/nodes\[3\]\.node.get_our_node_id\(\)/node_d_id/g' \
-e 's/nodes\[4\]\.node.get_our_node_id\(\)/node_e_id/g' \
-e 's/nodes\[5\]\.node.get_our_node_id\(\)/node_f_id/g' \
-e 's/(^.*create_network\(2, .*\);)/\1\nlet node_a_id = nodes\[0\].node.get_our_node_id\(\);\nlet node_b_id = nodes\[1\].node.get_our_node_id\(\);/g' \
-e 's/(^.*create_network\(3, .*\);)/\1\nlet node_a_id = nodes\[0\].node.get_our_node_id\(\);\nlet node_b_id = nodes\[1\].node.get_our_node_id\(\);\nlet node_c_id = nodes\[2\].node.get_our_node_id\(\);/g' \
-e 's/(^.*create_network\(4, .*\);)/\1\nlet node_a_id = nodes\[0\].node.get_our_node_id\(\);\nlet node_b_id = nodes\[1\].node.get_our_node_id\(\);\nlet node_c_id = nodes\[2\].node.get_our_node_id\(\);\nlet node_d_id = nodes\[3\].node.get_our_node_id\(\);/g' \
-e 's/(^.*create_network\(5, .*\);)/\1\nlet node_a_id = nodes\[0\].node.get_our_node_id\(\);\nlet node_b_id = nodes\[1\].node.get_our_node_id\(\);\nlet node_c_id = nodes\[2\].node.get_our_node_id\(\);\nlet node_d_id = nodes\[3\].node.get_our_node_id\(\);\nlet node_e_id = nodes\[4\].node.get_our_node_id\(\);/g' \
-e 's/(^.*create_network\(6, .*\);)/\1\nlet node_a_id = nodes\[0\].node.get_our_node_id\(\);\nlet node_b_id = nodes\[1\].node.get_our_node_id\(\);\nlet node_c_id = nodes\[2\].node.get_our_node_id\(\);\nlet node_d_id = nodes\[3\].node.get_our_node_id\(\);\nlet node_e_id = nodes\[4\].node.get_our_node_id\(\);\nlet node_f_id = nodes\[5\].node.get_our_node_id\(\);/g' \
"$file"
done
cargo +1.63.0 fmt
joostjager
force-pushed
the
rustfmt-async-kv-store
branch
from
2c2415e to
d507251
Compare
|
Rebased |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
LGTM
Checked rustfmt commits and the script outputs match, and reviewed the rest of the commits.
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Quite a few things worth cleaning up after this, but can be done in a followup.
#3882 post-rustfmt cleanups
4 participants
Rustfmt preparatory PR for #3778.
My view on the changes is again that refactors, however trivial, are risky. Especially with large quantities of similar edits. In this PR I didn't do any touch ups for that reason.
Let me know what pressing, non-subjective formatting problems are really unacceptable. I can add a commit for those where I either change the formatting, or simpliy rustfmt::skip the section.