Skip to content

Conversation

Sjors
Copy link
Member

@Sjors Sjors commented Jan 16, 2020

This was part of the abandoned #15150.

@Sjors Sjors requested a review from promag January 16, 2020 13:30
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK c279a81
Changes LGTM -- the variable wallet_model is not needed if ENABLE_WALLET is not set and accessing the current element of a combox-box using QComboBox::currentData() instead of using the index is possible since Qt 5.2 (https://doc.qt.io/qt-5/qcombobox.html#currentData-prop).

@promag
Copy link
Contributor

promag commented Jan 16, 2020

Thanks @Sjors for picking this, ACK.

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c279a81 - tested wallet loading/unloading in the qt rpc console.

This warning is fixed when compiling with --disable-wallet:

qt/rpcconsole.cpp:908:22: warning: unused variable 'wallet_model' [-Wunused-variable]
        WalletModel* wallet_model{nullptr};
                     ^
1 warning generated.

fanquake added a commit that referenced this pull request Jan 17, 2020
c279a81 gui: Remove warning "unused variable 'wallet_model'" (João Barbosa)

Pull request description:

  This was part of the abandoned #15150.

ACKs for top commit:
  theStack:
    utACK c279a81
  fanquake:
    ACK c279a81 - tested wallet loading/unloading in the qt rpc console.

Tree-SHA512: 8fbd55c7e213599c7be843b52e960a16cf965b3e01489f426ac3ed9d579d78bb4b2ac230bcccd8abe0397a8b1166ee10e0d685738441a77a5dcb5135c15790fa
@fanquake fanquake merged commit c279a81 into bitcoin:master Jan 17, 2020
@Sjors Sjors deleted the 2020/01/disable-wallet-unused-variable branch January 17, 2020 08:16
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Apr 6, 2020
- gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged bitcoin#18160
- gui: Drop PeerTableModel dependency to ClientModel bitcoin#18060
- gui: Break trivial circular dependencies bitcoin#18036
- gui: Improve "Hide" button tool-tip message bitcoin#17360
- gui: Shortcut to close ModalOverlay bitcoin#17998
- gui: Remove warning "unused variable 'wallet_model'" bitcoin#17939
- refactor: Use PACKAGE_NAME in GUI modal overlay and bitcoin-wallet bitcoin#17923
- gui: remove OpenSSL PRNG seeding (Windows, Qt only) bitcoin#17151
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Apr 6, 2020
- gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged bitcoin#18160
- gui: Drop PeerTableModel dependency to ClientModel bitcoin#18060
- gui: Break trivial circular dependencies bitcoin#18036
- gui: Improve "Hide" button tool-tip message bitcoin#17360
- gui: Shortcut to close ModalOverlay bitcoin#17998
- gui: Remove warning "unused variable 'wallet_model'" bitcoin#17939
- refactor: Use PACKAGE_NAME in GUI modal overlay and bitcoin-wallet bitcoin#17923
- gui: remove OpenSSL PRNG seeding (Windows, Qt only) bitcoin#17151
- refactor: Remove unused defines in qt/bitcoinunits.h bitcoin#17869
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 7, 2020
Summary:
For Core, this commit removed a warning when building without the wallet, because the wallet_model nullptr was defined outside of the `#ifdef ENABLE_WALLET` context. This problem was already fixed by D4661 in Bitcoin ABC.

So this is a simple refactoring that simplifies the code.

Quote from the PR conversation:
> accessing the current element of a combox-box using QComboBox::currentData() instead of using the index is possible since Qt 5.2

This is a backport of Core [[bitcoin/bitcoin#17939 | PR17939]]

Test Plan:
`ninja && ninja check && src/qt/bitcoin-qt`

In the RPC console, type a wallet related command and press enter while the line edit is the active widget.

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D8619
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 17, 2021
…el'"

c279a81 gui: Remove warning "unused variable 'wallet_model'" (João Barbosa)

Pull request description:

  This was part of the abandoned bitcoin#15150.

ACKs for top commit:
  theStack:
    utACK bitcoin@c279a81
  fanquake:
    ACK c279a81 - tested wallet loading/unloading in the qt rpc console.

Tree-SHA512: 8fbd55c7e213599c7be843b52e960a16cf965b3e01489f426ac3ed9d579d78bb4b2ac230bcccd8abe0397a8b1166ee10e0d685738441a77a5dcb5135c15790fa
thelazier pushed a commit to thelazier/dash that referenced this pull request Sep 25, 2021
…el'"

c279a81 gui: Remove warning "unused variable 'wallet_model'" (João Barbosa)

Pull request description:

  This was part of the abandoned bitcoin#15150.

ACKs for top commit:
  theStack:
    utACK bitcoin@c279a81
  fanquake:
    ACK c279a81 - tested wallet loading/unloading in the qt rpc console.

Tree-SHA512: 8fbd55c7e213599c7be843b52e960a16cf965b3e01489f426ac3ed9d579d78bb4b2ac230bcccd8abe0397a8b1166ee10e0d685738441a77a5dcb5135c15790fa
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants