gui, refactor: Register Qt meta types in application constructor #19104

pull promag wants to merge 1 commits into bitcoin:master from promag:2020-05-register-metatypes changing 1 files +18 −14

promag commented at 9:08 AM on May 29, 2020: member

Removes a warning when running QT_QPA_PLATFORM=cocoa src/qt/test/test_bitcoin-qt.
fanquake added the label GUI on May 29, 2020
promag commented at 9:10 AM on May 29, 2020: member

Pushed in #17457, then #19033, and now on it's own PR.
jonasschnelli commented at 9:12 AM on May 29, 2020: contributor

We could share those registrations with the main GUI code as also commented here. Since its test only,... not really necessary but would prevent further testing issues.

utACK 41c3af9fecd52d44fb205edec7c0f054036d41eb
promag commented at 9:14 AM on May 29, 2020: member

@jonasschnelli yes, was already suggested by @ryanofsky somewhere and I've suggested adding GUIUtils::registerMetaTypes.

Ah just saw you linked the suggestion comment.

promag commented at 10:23 AM on May 29, 2020: member

Moving to GUIUtil::registerMetaTypes, and because of WalletModel* registration, I need to #include <qt/walletmodel.h> which causes

A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/guiutil" appears to have been introduced.
A new circular dependency in the form of "qt/clientmodel -> qt/guiutil -> qt/walletmodel -> qt/clientmodel" appears to have been introduced.
A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/optionsmodel -> qt/guiutil" appears to have been introduced.
A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/paymentserver -> qt/guiutil" appears to have been introduced.

ryanofsky commented at 10:42 AM on May 29, 2020: member

re: #19104 (comment)

Slight preference for just adding a new file like src/qt/types.h or src/qt/util_types.h to adding new circular dependencies here
promag commented at 10:44 AM on May 29, 2020: member

How about BitcoinApplication::registerMetaTypes - maybe called from constructor?
ryanofsky commented at 10:50 AM on May 29, 2020: member

How about BitcoinApplication::registerMetaTypes - maybe called from constructor?

Seems good
promag force-pushed on May 29, 2020
promag renamed this:
~~gui, test: Fix Cannot queue arguments of type size_t~~
gui, refactor: Register Qt meta types in application constructor
on May 29, 2020
promag commented at 11:32 AM on May 29, 2020: member

Updated, almost move only.
jonasschnelli commented at 11:39 AM on May 29, 2020: contributor

utACK 9df2528e57ff3c22f3d5777eb6a60a326c909995
hebasto commented at 11:45 AM on May 29, 2020: member

Concept ACK.
in src/qt/bitcoin.cpp:257 in 9df2528e57 outdated
```
 253 | @@ -253,6 +254,23 @@ bool BitcoinApplication::baseInitialize()
 254 |      return m_node.baseInitialize();
 255 |  }
 256 |  
 257 | +void BitcoinApplication::registerMetaTypes()
```
hebasto commented at 2:54 PM on May 29, 2020:

Why a member function? Could it be a non-member function, either in unnamed namespace or static?

promag commented at 3:06 PM on May 29, 2020:

Good question 🤷 updated.
hebasto changes_requested
hebasto commented at 2:54 PM on May 29, 2020: member

Tested 9df2528e57ff3c22f3d5777eb6a60a326c909995 on macOS 10.15.5 -- works as expected.
gui, refactor: Register Qt meta types in application constructor 4f49d5222e
promag force-pushed on May 29, 2020
hebasto approved
hebasto commented at 3:16 PM on May 29, 2020: member

ACK 4f49d5222eca11c149713ad34113d5a3d1c577b1, tested on macOS 10.15.5.
DrahtBot commented at 4:35 PM on May 29, 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:
- #19011 (Reduce cs_main lock accumulation during GUI startup by jonasschnelli)
- #18817 (doc: Document differences in bitcoind and bitcoin-qt locale handling by practicalswift)
- #18604 (Display command line usage to console without requiring X Windows by rebroad)
- #17877 (qt, refactor: Make enums in BitcoinUnits class scoped 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.
jonasschnelli commented at 6:47 AM on June 2, 2020: contributor

Re utACK 4f49d5222eca11c149713ad34113d5a3d1c577b1
jonasschnelli merged this on Jun 2, 2020
jonasschnelli closed this on Jun 2, 2020
sidhujag referenced this in commit 042590c325 on Jun 2, 2020
promag deleted the branch on Jun 7, 2020
ComputerCraftr referenced this in commit d30a975a2c on Jun 10, 2020
Fabcien referenced this in commit 8744502667 on Jul 6, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

gui, refactor: Register Qt meta types in application constructor #19104

Conflicts