qt, refactor: Remove never used default parameter #17943

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:20200116-message-parameter changing 4 files +6 −15

hebasto commented at 6:52 PM on January 16, 2020: member

In BitcoinGUI::message() slot the bool* ret = nullptr parameter is never used.

This PR removes it and simplifies connections syntax by replacing lambdas with the &BitcoinGUI::message slot.
refactor: Remove never used default parameter 7d0a8f4f53
refactor: Simplify connection syntax 1a53b0da60
promag commented at 7:22 PM on January 16, 2020: member

Concept ACK, when was the last usage dropped?
DrahtBot added the label GUI on Jan 16, 2020
DrahtBot commented at 8:59 PM 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:
- #16224 (gui: Bilingual GUI error messages 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.
fanquake added the label Refactoring on Jan 16, 2020
Empact commented at 10:06 PM on January 16, 2020: member

The use was introduced in f7f3a96b74bb795d6e184a628adce21c744d234f and I believe removed in 35ecf854c084c248ad640c6af030a9d1ed726c47 but haven't built the old code to confirm.
Empact commented at 10:09 PM on January 16, 2020: member

Code review ACK https://github.com/bitcoin/bitcoin/pull/17943/commits/1a53b0da60097cd7fd423c519f01ceca0fd0aa14
promag commented at 1:20 AM on January 17, 2020: member

Code review ACK 1a53b0da60097cd7fd423c519f01ceca0fd0aa14.
Sjors commented at 9:21 AM on January 17, 2020: member

Tested ACK 1a53b0da60097cd7fd423c519f01ceca0fd0aa14

You can see messages in action by e.g. feeding a malformed BIP20 URI: src/qt/bitcoin-qt -regtest bitcoin://bcrt1q
fanquake referenced this in commit 0deba68064 on Jan 17, 2020
fanquake merged this on Jan 17, 2020
fanquake closed this on Jan 17, 2020
hebasto deleted the branch on Jan 18, 2020
hebasto commented at 6:36 PM on January 18, 2020: member

Sorry, It seems I broke ThreadSafeMessageBox() ((

bool* ret actually is used here: https://github.com/bitcoin/bitcoin/blob/a654626f076a72416a3d354218d7107571d6caaf/src/qt/bitcoingui.cpp#L1368

How it could be reverted?

Ref: #16348
promag commented at 7:35 PM on January 18, 2020: member

Please don't revert, I think the code you referenced is broken.

hebasto commented at 6:09 AM on January 19, 2020: member

@fanquake (from IRC):

It's unclear from your comments what is supposedly broken or not.

$ ./src/qt/bitcoin-qt 
: You need to rebuild the database using -reindex to go back to unpruned mode.  This will redownload the entire blockchain.
Please restart with -reindex or -reindex-chainstate to recover.
bitcoin-qt: qt/bitcoingui.cpp:1369: bool ThreadSafeMessageBox(BitcoinGUI*, const string&, const string&, unsigned int): Assertion `invoked' failed.
Aborted (core dumped)

laanwj referenced this in commit 0038e536de on Jan 22, 2020
PastaPastaPasta referenced this in commit 10185acfe8 on Dec 22, 2021
PastaPastaPasta referenced this in commit 8af1abab0e on Dec 22, 2021
PastaPastaPasta referenced this in commit 0a10713bb3 on Dec 22, 2021
PastaPastaPasta referenced this in commit 70c48b9ca0 on Dec 22, 2021
PastaPastaPasta referenced this in commit b6147764d0 on Dec 22, 2021
PastaPastaPasta referenced this in commit 3e28512f96 on Dec 22, 2021
PastaPastaPasta referenced this in commit 5dbec5163e on Dec 28, 2021
PastaPastaPasta referenced this in commit a60f648ed7 on Dec 28, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#17965 qt: Revert changes of pr17943

qt, refactor: Remove never used default parameter #17943

Conflicts