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
Fixup build with qt6 #3
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I'd also first introduce a version using new api provided by Qt 5.14.
resourceqt-client/client.cpp
Outdated
| @@ -29,6 +29,12 @@ USA. | |||
|
|
|||
| #include "client.h" | |||
|
|
|||
| #if (QT_VERSION < QT_VERSION_CHECK(6, 0, 0)) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5.14
| @@ -35,7 +36,11 @@ ResourceSet::ResourceSet(const QString &applicationClass, QObject * parent, | |||
| audioResource(NULL), autoRelease(initialAutoRelease), | |||
| alwaysReply(initialAlwaysReply), initialized(false), pendingAcquire(false), | |||
| pendingUpdate(false), pendingAudioProperties(false), pendingVideoProperties(false), | |||
| #if (QT_VERSION > QT_VERSION_CHECK(6,0,0)) | |||
| inAcquireMode(false), reqMutex(QRecursiveMutex()), ignoreQ(false) | |||
| #else | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just include the differing variables inside #if. inAcquireMode and ignoreQ don't need differences so could be taken out of here. Also the default ctor for the QRecursiveMutex could be skipped.
-> could just have "if < 5.14" without else branch.
Same on the other ctor.
| QMutexLocker locker(&mutex); | ||
| #else | ||
| mutex.lock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not following why this change? Different behavior too, nothing releases the mutex when the function is exited? Doesn't feel proper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still pending review comments here.
No description provided.