refactor: Optimize signal-slot connections logic #29

1. 
   hebasto commented at 9:44 am on July 10, 2020: member

   This PR:

   • implements an idea from https://github.com/bitcoin/bitcoin/pull/17937#issuecomment-575991765
   • simplifies WalletFrame class interface
   • as a side effect, removes bitcoingui -> walletframe -> bitcoingui circular dependency
   • is an alternative to https://github.com/bitcoin/bitcoin/pull/17500
2. 
   hebasto commented at 9:48 am on July 10, 2020: member

   This PR was originally opened in the https://github.com/bitcoin/bitcoin repo: https://github.com/bitcoin/bitcoin/pull/17966 has a concept ACK from @jonasschnelli.
3. hebasto closed this on Jul 14, 2020

   ---

4. hebasto reopened this on Jul 14, 2020

   ---

5. MarcoFalke added the label Refactoring on Aug 26, 2020
6. 
   jonasschnelli commented at 9:32 am on October 23, 2020: contributor

   Concept ACK
7. DrahtBot added the label Needs rebase on Oct 24, 2020
8. hebasto marked this as a draft on Oct 24, 2020
9. MarcoFalke referenced this in commit c857148636 on May 15, 2021
10. hebasto force-pushed on May 15, 2021
11. hebasto marked this as ready for review on May 15, 2021
12. 
    hebasto commented at 9:49 pm on May 15, 2021: member

    Updated 3eb6d5b4068194f3420a9f8b30cffd210ca7265b -> e7c7fbe9259c19ce55faa7397d5998b27d851224 (pr29.01 -> pr29.02):

    • rebased
    • some commits are split into #333
13. DrahtBot removed the label Needs rebase on May 15, 2021
14. qt: Move WalletView connections from WalletFrame to BitcoinGUI

    This changes remove some pointers to the BitcoinGUI instance that is
    required for the next commits.
    
    This commit does not change behavior.

    20e2e24e90
15. qt: Move CreateWalletActivity connection from WalletFrame to BitcoinGUI

    This changes remove some pointers to the BitcoinGUI instance that is
    required for the next commits.
    
    This commit does not change behavior.

    f73e5c972a
16. qt: Drop BitcoinGUI* WalletFrame data member

    This changes removes bitcoingui->walletframe->bitcoingui circular
    dependency.
    
    This commit does not change behavior.

    62cb8d98d2
17. hebasto force-pushed on May 29, 2021
18. 
    hebasto commented at 12:51 pm on May 29, 2021: member

    Rebased e7c7fbe9259c19ce55faa7397d5998b27d851224 -> 62cb8d98d27e7f316f01f177f35ad0ed6f8cd9ce (pr29.02 -> pr29.03) on top of the recent CI changes.
19. 
    promag commented at 8:46 pm on June 2, 2021: contributor

    Code review ACK 62cb8d98d27e7f316f01f177f35ad0ed6f8cd9ce. Will build and test.

    Nice to see you picked https://github.com/bitcoin/bitcoin/pull/17500#issuecomment-575088028 suggestion.

20. in src/qt/bitcoingui.cpp:692 in 62cb8d98d2

    687@@ -680,6 +688,18 @@ void BitcoinGUI::addWallet(WalletModel* walletModel)
    688     }
    689     const QString display_name = walletModel->getDisplayName();
    690     m_wallet_selector->addItem(display_name, QVariant::fromValue(walletModel));
    691+
    692+    connect(wallet_view, &WalletView::outOfSyncWarningClicked, walletFrame, &WalletFrame::outOfSyncWarningClicked);
    

    ---

    ---

    

    promag commented at 2:49 pm on June 3, 2021:

    nit, maybe this should stay in WalletFrame?

    ---

    

    hebasto commented at 11:25 am on June 5, 2021:

    Doing in this way keeps compatibility with #333.
21. 
    promag commented at 2:50 pm on June 3, 2021: contributor

    Tested ACK 62cb8d98d27e7f316f01f177f35ad0ed6f8cd9ce on macos 11.2.3 with depends build.

    Ran with QT_FATAL_WARNINGS=0, didn’t observe behavior changes.

22. 
    jarolrod commented at 9:30 pm on June 5, 2021: member

    ACK 62cb8d98d27e7f316f01f177f35ad0ed6f8cd9ce

    Tested that there are no weird side-effects. Also used @promag instructions to look for behavior changes aside from manual testing. Ran on Arch Linux Qt 5.15.2, macOS 11.3 Qt 5.15.2 and cross-compile to Windows 10.

    Overall, I think the changes in this PR provide a clearer definition of the functionality the walletframe should be responsible for performing vs being aware of. Thanks for working on this.

23. hebasto merged this on Jun 5, 2021
24. hebasto closed this on Jun 5, 2021

    ---

25. hebasto deleted the branch on Jun 5, 2021
26. sidhujag referenced this in commit 1308761b50 on Jun 6, 2021
27. hebasto referenced this in commit 6f3fbc062f on Jun 12, 2021
28. hebasto referenced this in commit e9472e6828 on Aug 5, 2021
29. gwillen referenced this in commit 69fd180a34 on Jun 1, 2022
30. bitcoin-core locked this on Aug 16, 2022

Contributors
hebasto jonasschnelli promag jarolrod

Labels
Refactoring