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

DolphinQt: Use qOverload where applicable #8272

Open
wants to merge 1 commit into
base: master
from

Conversation

@lioncash
Copy link
Member

commented Jul 30, 2019

Provides the same behvaior, but in a much more concise manner.

@lioncash

This comment has been minimized.

Copy link
Member Author

commented Jul 30, 2019

Windows buildbot seems to have a version of Qt less than 5.7, which would put it around early 2016. It may be worth updating it to 5.11 or any other newer version.

@BhaaLseN

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

Any reason to not simply use QOverload<T>::of(&QType::method) instead of qOverload<T>(&QType::method)? We use the former in other places already (like this one:

connect(m_slot_combos[i], QOverload<int>::of(&QComboBox::currentIndexChanged), this,
[this, i] { UpdateButton(i); });
connect(m_slot_combos[i], QOverload<int>::of(&QComboBox::currentIndexChanged), this,
&GameCubePane::SaveSettings);
), and it only differs by 4 characters.

DolphinQt: Use qOverload where applicable
Provides the same behvaior, but in a much more concise manner.

@lioncash lioncash force-pushed the lioncash:overload branch from 346c2a7 to 593c2c6 Jul 30, 2019

@lioncash

This comment has been minimized.

Copy link
Member Author

commented Jul 30, 2019

It's (now) intended as a stopgap for C++11-only code that is unable to use C++14. While it's an option, I do think the buildbot should be updated to use roughly the same version of the Qt libraries that is used in our ext-win-qt dependency, which provides the equivalent of 5.11.1.

It's quite odd to remain on an old version of Qt for the buildbots, but not actually use that version in our own dependencies.

@BhaaLseN

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

I do wonder about that as well...it should be using it, since there is no installation of Qt on them (at least to my knowledge - and if there is, we should probably remove it?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.