-
Notifications
You must be signed in to change notification settings - Fork 37.7k
gui: Remove WalletView and BitcoinGUI circular dependency #17937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gui: Remove WalletView and BitcoinGUI circular dependency #17937
Conversation
ac36752
to
ba15dae
Compare
ba15dae
to
72b5a23
Compare
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Concept ACK. |
Concept ACK |
72b5a23
to
33727e0
Compare
w.r.t. #17500 (comment) It seems a bit of formalization could be helpful for our community. How about: Qt Slots and Signals |
@hebasto looks good! |
33727e0
to
cb8a86d
Compare
Rebased and reverted to previous version since #17965 was merged. |
Appveyor status is wrong, it actually passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK cb8a86d, tested on Linux Mint 19.3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK cb8a86d
cb8a86d gui: Remove WalletView and BitcoinGUI circular dependency (João Barbosa) ac3d107 gui: Add transactionClicked and coinsSent signals to WalletView (João Barbosa) Pull request description: 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. ACKs for top commit: hebasto: ACK cb8a86d, tested on Linux Mint 19.3. jonasschnelli: utACK cb8a86d Tree-SHA512: 250316cd3689e51c8cded9ccd75963c836dcafa6db25d684f2aa691dea9738895f9140793e0f925784909e39f8257f7e1c7d611e8bd6d6634e1a50333f4ddb1e
…ependency cb8a86d gui: Remove WalletView and BitcoinGUI circular dependency (João Barbosa) ac3d107 gui: Add transactionClicked and coinsSent signals to WalletView (João Barbosa) Pull request description: 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. ACKs for top commit: hebasto: ACK cb8a86d, tested on Linux Mint 19.3. jonasschnelli: utACK cb8a86d Tree-SHA512: 250316cd3689e51c8cded9ccd75963c836dcafa6db25d684f2aa691dea9738895f9140793e0f925784909e39f8257f7e1c7d611e8bd6d6634e1a50333f4ddb1e
Fix intro dialog labels when the prune button is toggled (bitcoin#17453) Rename debug window (bitcoin#17096) Add close window shortcut (bitcoin#15768) Add privacy to the Overview page (bitcoin#16432) Remove WalletView and BitcoinGUI circular dependency (bitcoin#17937) Force set nPruneSize in QSettings after the intro dialog (bitcoin#17696)
…ependency cb8a86d gui: Remove WalletView and BitcoinGUI circular dependency (João Barbosa) ac3d107 gui: Add transactionClicked and coinsSent signals to WalletView (João Barbosa) Pull request description: 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. ACKs for top commit: hebasto: ACK cb8a86d, tested on Linux Mint 19.3. jonasschnelli: utACK cb8a86d Tree-SHA512: 250316cd3689e51c8cded9ccd75963c836dcafa6db25d684f2aa691dea9738895f9140793e0f925784909e39f8257f7e1c7d611e8bd6d6634e1a50333f4ddb1e
Summary: > Two new signals are added beforehand in the first commit so that the connections in WalletFrame are all from the wallet view. This is a backport of Core [[bitcoin/bitcoin#17937 | PR17937]] [1/2] bitcoin/bitcoin@ac3d107 Test Plan: `ninja && src/qt/bitcoin-qt` Send a transaction, check that the transaction history page is automatically selected Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8698
Summary: > Essentially moves the code in WalletView::setBitcoinGUI to the only caller. Remove the circular dependency "qt/bitcoingui -> qt/walletview -> qt/bitcoingui" This is a backport of Core [[bitcoin/bitcoin#17937 | PR17937]] [2/2] bitcoin/bitcoin@cb8a86d Depends on D8698 Test Plan: `ninja && src/qt/bitcoin-qt` Test some of the connections Run `test/lint/lint-circular-dependencies.sh ` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8699
62cb8d9 qt: Drop BitcoinGUI* WalletFrame data member (Hennadii Stepanov) f73e5c9 qt: Move CreateWalletActivity connection from WalletFrame to BitcoinGUI (Hennadii Stepanov) 20e2e24 qt: Move WalletView connections from WalletFrame to BitcoinGUI (Hennadii Stepanov) Pull request description: This PR: - implements an idea from bitcoin/bitcoin#17937 (comment) - simplifies `WalletFrame` class interface - as a side effect, removes `bitcoingui` -> `walletframe` -> `bitcoingui` circular dependency - is an alternative to bitcoin/bitcoin#17500 ACKs for top commit: promag: Tested ACK 62cb8d9 on macos 11.2.3 with depends build. jarolrod: ACK 62cb8d9 Tree-SHA512: 633b526a8499ba9ab4b16928daf4de4f6d610284bb9fa51891cad35300a03bde740df3466a71b46e87a62121330fcc9e606eac7666ea5e45fa6d5785b60dcbbd
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 inWalletFrame
are all from the wallet view.