test: follow up for #23316 #23327

pull stratospher wants to merge 1 commits into bitcoin:master from stratospher:for-loop-init_wallet changing 1 files +2 −3

stratospher commented at 6:48 pm on October 20, 2021: contributor

This PR addresses #23316(review) and uses a for loop to make init_wallet() calls in test/functional/wallet_backup.py.
use for loop to perform init_wallet() calls 282386131c
MarcoFalke commented at 6:49 pm on October 20, 2021: member

-0 Looks ok, but doesn’t look like a clear improvement either
jonatack commented at 6:59 pm on October 20, 2021: member

-0 Looks ok, but doesn’t look like a clear improvement either

Agree, on its own not worth making a pull request. It’s ok for tests to be simple code. I apologize if my review comment made this change appear PR-worthy on its own.
MarcoFalke commented at 7:04 pm on October 20, 2021: member
Thank you for your contribution. While this stylistic change makes sense on its own, it comes at a cost and risk for the project as a whole. The weak motivation for the change does not justify the burden that it places on the project. A burden could be any of the following:
- Time spent on review
- Accidental introduction of bugs
- (Silent) merge conflicts, either in the branch or a backport branch. Those conflicts demand further developer and reviewer time or introduce bugs.
For more information about refactoring changes and stylistic cleanup, see
Generally, if the style is not mentioned nor enforced by the developer notes, we leave it up to the original author to pick whatever fits them best personally and then leave it that way until the line is touched for other reasons.

Let me know if you have any questions.
MarcoFalke closed this on Oct 20, 2021
stratospher deleted the branch on Oct 21, 2021
DrahtBot locked this on Oct 30, 2022

Contributors
stratospher MarcoFalke jonatack