Skip to content
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

refactor: Use template function qOverload in signal-slot connections #257

Merged
merged 1 commit into from
May 10, 2021

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Mar 24, 2021

A nice template function qOverload is available for us now (bitcoin/bitcoin#20413, bitcoin/bitcoin#21286).

Its usage makes code much more readable.

This PR does not change behavior.

@hebasto hebasto closed this Mar 24, 2021
@hebasto hebasto reopened this Mar 24, 2021
@hebasto hebasto closed this Apr 3, 2021
@hebasto hebasto reopened this Apr 3, 2021
Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK c8169f4

The use of qOverload is correct and is a nice simplication. Tested that behavior remains the same.

@hebasto
Copy link
Member Author

hebasto commented Apr 29, 2021

@Talkless

wrt your comment:

IIRC qOverload function template does not work with Visual Studio 2017 due to implementation limitations...

have you any update?

@hebasto
Copy link
Member Author

hebasto commented Apr 29, 2021

Rebased c8169f4 -> cdbc2bd (pr257.01 -> pr257.02) due to the conflict with #125.

@hebasto
Copy link
Member Author

hebasto commented May 2, 2021

@Talkless

IIRC qOverload function template does not work with Visual Studio 2017 due to implementation limitations...

That is not the case now since bitcoin/bitcoin#21811 is merged.

Copy link

@Talkless Talkless left a comment

Choose a reason for hiding this comment

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

utACK cdbc2bd.

I did build on Debian Sid and Debian 10 Buster but did not check any signals in running program. All changes looks clear. Nice upgrade.

@hebasto hebasto requested a review from promag May 8, 2021 06:23
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Code review ACK cdbc2bd.

Thanks for also updating QOverload<T>::of in intro.cpp.

@hebasto hebasto merged commit 8d7125f into bitcoin-core:master May 10, 2021
@hebasto hebasto deleted the 210323-overload branch May 10, 2021 20:44
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 11, 2021
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 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