gui: Remove WalletView and BitcoinGUI circular dependency #17937

1. 
   promag commented at 11:06 pm on January 15, 2020: member

   Essentially moves the code in WalletView::setBitcoinGUI to the only caller. Two new signals are added beforehand in the first commit so that the connections in WalletFrame are all from the wallet view.
2. promag force-pushed on Jan 15, 2020
3. fanquake added the label GUI on Jan 15, 2020
4. promag requested review from hebasto on Jan 15, 2020
5. promag force-pushed on Jan 16, 2020
6. 
   DrahtBot commented at 2:55 am on January 16, 2020: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #16432 (qt: Add privacy to the Overview page by hebasto)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

7. promag requested review from jonasschnelli on Jan 16, 2020
8. 
   hebasto commented at 10:10 am on January 16, 2020: member

   Concept ACK.
9. in src/qt/walletframe.cpp:68 in 72b5a2343c outdated

   60@@ -62,6 +61,14 @@ bool WalletFrame::addWallet(WalletModel *walletModel)
   61     mapWalletViews[walletModel] = walletView;
   62 
   63     connect(walletView, &WalletView::outOfSyncWarningClicked, this, &WalletFrame::outOfSyncWarningClicked);
   64+    connect(walletView, &WalletView::transactionClicked, gui, &BitcoinGUI::gotoHistoryPage);
   65+    connect(walletView, &WalletView::coinsSent, gui, &BitcoinGUI::gotoHistoryPage);
   66+    connect(walletView, &WalletView::message, [this](const QString& title, const QString& message, unsigned int style) {
   67+        gui->message(title, message, style);
   68+    });
   

   ---

   ---

   

   hebasto commented at 10:36 am on January 16, 2020:

   s/[this]/[gui]/ ?

   ---

   

   promag commented at 10:42 am on January 16, 2020:

   Doesn’t work, gui is a member.

   ---

   

   hebasto commented at 10:53 am on January 16, 2020:

   oops

   BitcoinGUI::message is a slot, and there is no need to capture local variable gui. Why not use non-lambda connection syntax?

   nm: it does not work too, but could work if remove unused default parameter from BitcoinGUI::message signature.

   ---

   

   promag commented at 11:05 am on January 16, 2020:

   Right, originally couldn’t connect to slot because of that. I think we can “clean” these later.
10. hebasto changes_requested
11. 
    Empact commented at 9:52 pm on January 16, 2020: member

    Concept ACK
12. promag force-pushed on Jan 17, 2020
13. 
    promag commented at 1:19 am on January 17, 2020: member

    To avoid conflicts rebased on #17943 which has already @Empact ACK (and mine fwiw).
14. 
    hebasto commented at 10:48 am on January 19, 2020: member

    w.r.t. #17500 (comment)

    It seems a bit of formalization could be helpful for our community. How about:

    Qt Slots and Signals Do no make connections to external signals or slots in the QObject subclass implementation. Rationale: Do not introduce needless dependencies.

15. 
    promag commented at 10:51 am on January 19, 2020: member

    @hebasto looks good!
16. DrahtBot added the label Needs rebase on Jan 22, 2020
17. gui: Add transactionClicked and coinsSent signals to WalletView ac3d10777d
18. gui: Remove WalletView and BitcoinGUI circular dependency cb8a86d9f9
19. promag force-pushed on Jan 30, 2020
20. 
    promag commented at 11:49 am on January 30, 2020: member

    Rebased and reverted to previous version since #17965 was merged.
21. DrahtBot removed the label Needs rebase on Jan 30, 2020
22. 
    promag commented at 4:15 pm on January 31, 2020: member

    Appveyor status is wrong, it actually passed.
23. hebasto approved
24. 
    hebasto commented at 4:29 pm on January 31, 2020: member

    ACK cb8a86d9f952401eaad68b2e3818ce50f7befd91, tested on Linux Mint 19.3.
25. jonasschnelli approved
26. 
    jonasschnelli commented at 9:10 am on February 1, 2020: contributor

    utACK cb8a86d9f952401eaad68b2e3818ce50f7befd91
27. jonasschnelli referenced this in commit f05c1ac444 on Feb 1, 2020
28. jonasschnelli merged this on Feb 1, 2020
29. jonasschnelli closed this on Feb 1, 2020

    ---

30. promag deleted the branch on Feb 1, 2020
31. sidhujag referenced this in commit c5b9ac8c8f on Feb 1, 2020
32. MarkLTZ referenced this in commit 136a27c84d on Feb 3, 2020
33. sidhujag referenced this in commit 0e7e1487b2 on Nov 10, 2020
34. Fabcien referenced this in commit f36cdc7e0c on Dec 18, 2020
35. Fabcien referenced this in commit 94bf18e8bf on Dec 19, 2020
36. hebasto referenced this in commit e033ca1379 on Jun 5, 2021
37. DrahtBot locked this on Feb 15, 2022

Contributors
promag DrahtBot hebasto Empact jonasschnelli

Labels
GUI