[qt] move SendCoinsRecipient to its own file #12433

pull Sjors wants to merge 1 commits into bitcoin:master from Sjors:2018/02/qt-send-coins-recipient changing 9 files +115 −69

Sjors commented at 8:46 PM on February 14, 2018: member

I plan to add some instance methods to SendCoinsRecipient in an upcoming PR, so this might be a good time to give it its own file.
fanquake added the label GUI on Feb 15, 2018
Sjors force-pushed on Feb 15, 2018
in src/qt/walletmodeltransaction.cpp:10 in de67ce6988 outdated
```
   6 | @@ -7,6 +7,8 @@
   7 |  #include <policy/policy.h>
   8 |  #include <wallet/wallet.h>
   9 |  
  10 | +class SendCoinsRecipient;
```
promag commented at 10:50 AM on February 16, 2018:

Remove?

Sjors commented at 4:46 PM on February 16, 2018:

Removed.

promag commented at 10:57 AM on February 16, 2018: member

IMO if it's really "necessary" to move to a separate file then that can happen in the PR that takes advantage of it.

Build job failed with

Making check in src
make[1]: Entering directory `/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/src'
make[2]: Entering directory `/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/src'
  OBJCXXLD qt/bitcoin-qt
  CXXLD    qt/test/test_bitcoin-qt
qt/libbitcoinqt.a(qt_libbitcoinqt_a-guiutil.o): In function `GUIUtil::parseBitcoinURI(QUrl const&, SendCoinsRecipient*)':
guiutil.cpp:(.text+0x557f): undefined reference to `SendCoinsRecipient::SendCoinsRecipient()'
qt/test/qt_test_test_bitcoin_qt-uritests.o: In function `URITests::uriTests()':
uritests.cpp:(.text+0x33): undefined reference to `SendCoinsRecipient::SendCoinsRecipient()'
collect2: error: ld returned 1 exit status

Sjors force-pushed on Feb 16, 2018
Sjors commented at 4:46 PM on February 16, 2018: member

The Travis instance that builds QT without the wallet broke, so I added a bunch of #ifdef ENABLE_WALLET to guiutil.cpp, for example around parseBitcoinURI. I should probably do that in a separate commit.

IMO if it's really "necessary" to move to a separate file then that can happen in the PR that takes advantage of it.

I know there's a (implicit?) policy against refactoring for the sake of refactoring. On the other hand, I like to keep my PR's short because I think it makes review easier. I can rebase my upcoming PR on top of this if people prefer that. Or we can hold off merging this until that PR is ready and has a good chance of getting merged.
[refactor][qt] move SendCoinsRecipient to its own file 370ebf2a23
Sjors force-pushed on Feb 26, 2018

Sjors commented at 4:59 PM on February 26, 2018: member

Not sure what to do about these has no symbols warnings:

  AR       qt/libbitcoinqt.a
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: qt/libbitcoinqt.a(qt_libbitcoinqt_a-moc_sendcoinsrecipient.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: qt/libbitcoinqt.a(qt_libbitcoinqt_a-moc_sendcoinsrecipient.o) has no symbols
  OBJCXXLD qt/bitcoin-qt

laanwj assigned jonasschnelli on Mar 6, 2018
MarcoFalke commented at 1:12 AM on March 19, 2018: member

Needs rebase if still relevant
in src/qt/openuridialog.cpp:35 in 370ebf2a23
```
  31 | @@ -32,6 +32,7 @@ QString OpenURIDialog::getURI()
  32 |  
  33 |  void OpenURIDialog::accept()
  34 |  {
  35 | +#ifdef ENABLE_WALLET
```
jonasschnelli commented at 2:30 AM on March 19, 2018:

Is this #ifdef necessary? IMO openuridialog.cpp/.h is only in BITCOIN_QT_WALLET_CPP.
jonasschnelli commented at 2:33 AM on March 19, 2018: contributor

Concept ACK... but you should compile the new class always (since its not really depending on wallet classes)... also, avoid additional #ifdefs (IMO they are the last resort for selective features).
fanquake commented at 1:57 AM on April 12, 2018: member

Going to close for now. Needs a rebase. I also somewhat agree that a change like this can be incorporated into the first PR that will be making use of it.
fanquake closed this on Apr 12, 2018
Sjors deleted the branch on Apr 12, 2018
Sjors commented at 9:03 AM on April 12, 2018: member

Ok, it also looks I'm going for a different approach for RBF than what I had in mind back then.
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels