util, refactor: Improve headers for bitcoin-wallet tool #23346

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:211024-bw-headers changing 3 files +16 −2

hebasto commented at 5:01 PM on October 24, 2021: member
This PR:
- removes unneeded #include <wallet/wallet.h> from <wallet/wallettool.h>
- introduces class forward declaration in <wallet/wallettool.h>
- added #include <config/bitcoin-config.h> to wallet/wallettool.cpp where the USE_BDB macro is used
DrahtBot added the label Utils/log/libs on Oct 24, 2021
DrahtBot added the label Wallet on Oct 24, 2021
DrahtBot commented at 5:34 PM on October 24, 2021: contributor



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



Conflicts

No conflicts as of last run.
DrahtBot cross-referenced this on Oct 24, 2021 from issue multiprocess: Add new bitcoin-gui, bitcoin-qt, bitcoin-wallet init implementations by ryanofsky
DrahtBot added the label Needs rebase on Oct 26, 2021
hebasto force-pushed on Oct 26, 2021
hebasto commented at 3:30 PM on October 26, 2021: member

Rebased 5fc7c3161e9d0d3ea7ca7a926fb09ddbd626cf32 -> 49b906c842676a79c16d5def7588ac7aefb72ad8 (pr23346.01 -> pr23346.02) due to the conflict with #23006.
DrahtBot cross-referenced this on Oct 26, 2021 from issue Multiprocess bitcoin by ryanofsky
DrahtBot removed the label Needs rebase on Oct 26, 2021
stratospher commented at 6:23 PM on October 27, 2021: contributor
ACK 49b906c. Verified that:
- #include <wallet/wallet.h> isn’t needed in src/wallet/wallettool.h
- All the newly added #include in the files are necessary.
I’m curious to know why WalletShowInfo() was moved out of src/wallet/wallettool.h though.
hebasto commented at 6:27 PM on October 27, 2021: member

I’m curious to know why WalletShowInfo() was moved out of src/wallet/wallettool.h though.

It is used in the only compilation unit: https://github.com/bitcoin/bitcoin/blob/49b906c842676a79c16d5def7588ac7aefb72ad8/src/wallet/wallettool.cpp#L100
util, refactor: Improve headers for bitcoin-wallet tool 3431839c33

in src/bitcoin-wallet.cpp:8 in 49b906c842 outdated

   1 | @@ -2,20 +2,23 @@
   2 |  // Distributed under the MIT software license, see the accompanying
   3 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 |  
   5 | -#if defined(HAVE_CONFIG_H)
   6 | -#include <config/bitcoin-config.h>
   7 | -#endif
   8 | -

luke-jr commented at 4:58 PM on October 29, 2021:

This should be left in IMO.

hebasto commented at 6:47 PM on October 30, 2021:

Updated.

hebasto force-pushed on Oct 30, 2021
hebasto commented at 6:46 PM on October 30, 2021: member
Updated 49b906c842676a79c16d5def7588ac7aefb72ad8 -> 3431839c33fa3892c982322e4add39e28ddba719 (pr23346.02 -> pr23346.03, diff):
- addressed @luke-jr's comment
DrahtBot cross-referenced this on Nov 2, 2021 from issue multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky
DrahtBot cross-referenced this on Nov 2, 2021 from issue multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky
Sjors commented at 10:39 AM on December 9, 2021: member

Lgtm, @achow101?
MarcoFalke merged this on Dec 9, 2021
MarcoFalke closed this on Dec 9, 2021

in src/wallet/wallettool.cpp:9 in 3431839c33

   1 | @@ -2,6 +2,12 @@
   2 |  // Distributed under the MIT software license, see the accompanying
   3 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 |  
   5 | +#if defined(HAVE_CONFIG_H)
   6 | +#include <config/bitcoin-config.h>
   7 | +#endif
   8 | +
   9 | +#include <wallet/wallettool.h>

MarcoFalke commented at 12:47 PM on December 9, 2021:

doesn't matter here, but the self-include is always the very first one on its own section for us, I think

hebasto deleted the branch on Dec 9, 2021
sidhujag referenced this in commit d07304d011 on Dec 10, 2021
RandyMcMillan referenced this in commit 8d6a2c5ca0 on Dec 23, 2021
bitcoin locked this on Dec 9, 2022

Contributors

Labels

Wallet Utils/log/libs

util, refactor: Improve headers for bitcoin-wallet tool #23346

Conflicts