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

Use the Qt5 connection syntax #6041

Merged
merged 6 commits into from
Sep 21, 2017
Merged

Use the Qt5 connection syntax #6041

merged 6 commits into from
Sep 21, 2017

Conversation

ogoffart
Copy link
Contributor

This is motivated by the fact that QMetaObject::noralizeSignature takes 7.35%
CPU of the LargeSyncBench. (Mostly from ABstractNetworkJob::setupConnections and
PropagateUploadFileV1::startNextChunk). It could be fixed by using normalized
signature in the connection statement, but i tought it was a good oportunity
to modernize the code.

One commit is done automatically, then i started doing the the rest is manually, altough not everything has been converted.

It's simpler, and QSignalMapper is deprecated in Qt 5.10
As we use the new connection syntax in folderman.cpp, some more symbol
need to be mocked
Copy link
Member

@jturcotte jturcotte left a comment

Choose a reason for hiding this comment

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

👍

This is motivated by the fact that QMetaObject::noralizeSignature takes 7.35%
CPU of the LargeSyncBench. (Mostly from ABstractNetworkJob::setupConnections and
PropagateUploadFileV1::startNextChunk). It could be fixed by using normalized
signature in the connection statement, but i tought it was a good oportunity
to modernize the code.

This commit only contains calls that were automatically converted with clazy.
Since we used the new syntax in connect, we need to use it in disconnect
Some slot were protected or private but needed to be public.
Some needed a static_cast (can't use qOverload because it is in Qt 5.7)

This is not only a partial change.
@ogoffart
Copy link
Contributor Author

(was rebased to fix conflicts)

@ogoffart ogoffart deleted the clazy branch September 21, 2017 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants