28950 followups #29722

1. 
   instagibbs commented at 12:34 pm on March 25, 2024: member

   Follow-ups to https://github.com/bitcoin/bitcoin/pull/28950
2. 
   DrahtBot commented at 12:34 pm on March 25, 2024: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	glozow, stickies-v

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. glozow added the label Refactoring on Mar 25, 2024
4. 
   glozow commented at 1:22 pm on March 25, 2024: member

   LGTM. Did you want to take #28950 (review)?
5. 
   instagibbs commented at 1:33 pm on March 25, 2024: member

   @glozow punting on that one, thanks
6. instagibbs force-pushed on Mar 25, 2024
7. 
   DrahtBot commented at 1:39 pm on March 25, 2024: contributor

   🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

   Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   Leave a comment here, if you need help tracking down a confusing failure.

   Debug: https://github.com/bitcoin/bitcoin/runs/23054231617

8. DrahtBot added the label CI failed on Mar 25, 2024
9. 
   fanquake commented at 2:54 pm on March 25, 2024: member

   0test/txpackage_tests.cpp:821:103: error: argument name 'client_maxfeerate' in comment does not match parameter name 'client_max_feerate' [bugprone-argument-comment,-warnings-as-errors]
   1  821 |                                                           package_rich_parent, /*test_accept=*/false, /*client_maxfeerate=*/{});
   2      |                                                                                                       ^~~~~~~~~~~~~~~~~~~~~~
   3      |                                                                                                       /*client_max_feerate=*/
   

10. 
    stickies-v commented at 3:25 pm on March 25, 2024: contributor

    Approach ACK cc2a4a6b8ec2c01ef4eed464c5173255d2ff7497. First commit should probably be a scripted-diff though, and I think the last 2 commits make more sense when squashed?

    LGTM. Did you want to take #28950 (review)?

    I think that’s fine to leave as is, as per #28950 (review)

11. instagibbs force-pushed on Mar 25, 2024
12. instagibbs force-pushed on Mar 25, 2024
13. instagibbs force-pushed on Mar 25, 2024
14. scripted-diff: Rename max_sane_feerate to client_maxfeerate

    -BEGIN VERIFY SCRIPT-
    git grep -l 'max_sane_feerate' | xargs sed -i 's/max_sane_feerate/client_maxfeerate/g'
    -END VERIFY SCRIPT-

    f10fd07320
15. use const ref for client_maxfeerate 7b29119d79
16. instagibbs force-pushed on Mar 25, 2024
17. 
    instagibbs commented at 3:59 pm on March 25, 2024: member

    First commit should probably be a scripted-diff though, and I think the last 2 commits make more sense when squashed?

    both done, thanks

18. 
    glozow commented at 5:06 pm on March 25, 2024: member

    utACK 7b29119d79efbc8c4148f350cc86531fde8b7251
19. DrahtBot requested review from stickies-v on Mar 25, 2024
20. DrahtBot removed the label CI failed on Mar 25, 2024
21. stickies-v approved
22. 
    stickies-v commented at 6:35 pm on March 25, 2024: contributor

    ACK 7b29119d79efbc8c4148f350cc86531fde8b7251
23. glozow merged this on Mar 26, 2024
24. glozow closed this on Mar 26, 2024

    ---

Contributors
instagibbs DrahtBot glozow fanquake stickies-v

Labels
Refactoring