D24014: [KWayland] Port away from deprecated API in Qt 5.14
dfaure closed this revision. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D24014 To: dfaure, davidedmundson, zzag Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24014: [KWayland] Port away from deprecated API in Qt 5.14
zzag accepted this revision. zzag added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > dfaure wrote in test_wayland_surface.cpp:396 > The "Almost Always Auto" crowd would disagree with you, but sure, your > choice. Done. Putting auto everywhere is a terrible thing! Surely it makes refactoring a bit easier, however you kill readability by erasing types. Qt has a really good auto keyword usage guideline. https://wiki.qt.io/Coding_Conventions#auto_Keyword REPOSITORY R127 KWayland BRANCH master REVISION DETAIL https://phabricator.kde.org/D24014 To: dfaure, davidedmundson, zzag Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24014: [KWayland] Port away from deprecated API in Qt 5.14
dfaure updated this revision to Diff 66290. dfaure marked 2 inline comments as done. dfaure added a comment. Less auto, more range-for, sorting includes REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24014?vs=66276&id=66290 BRANCH master REVISION DETAIL https://phabricator.kde.org/D24014 AFFECTED FILES autotests/client/test_wayland_surface.cpp src/client/region.cpp src/client/surface.cpp src/server/surface_interface_p.h src/tools/generator.cpp src/tools/testserver/testserver.cpp To: dfaure, davidedmundson, zzag Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24014: [KWayland] Port away from deprecated API in Qt 5.14
dfaure marked 3 inline comments as done.
dfaure added inline comments.
INLINE COMMENTS
> zzag wrote in test_wayland_surface.cpp:396
> I know it's unrelated, but erasing smart pointer types perhaps is not the
> greatest idea. Could you please use concrete type instead of auto while
> you're on this?
The "Almost Always Auto" crowd would disagree with you, but sure, your choice.
Done.
> zzag wrote in region.cpp:61
> Heh, you could make it a bit simpler:
>
> for (const QRect &rect : region) {
> ...
> }
Heh indeed. I looked at the API, saw begin/end, used begin/end I forgot
that this means range-for is available too :)
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D24014
To: dfaure, davidedmundson, zzag
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24014: [KWayland] Port away from deprecated API in Qt 5.14
zzag added inline comments.
INLINE COMMENTS
> test_wayland_surface.cpp:396
>
> -wl_buffer *blackBuffer = *(m_shm->createBuffer(black).data());
> -auto redBuffer = m_shm->createBuffer(red);
> +auto blackBufferPtr = m_shm->createBuffer(black).toStrongRef();
> +QVERIFY(blackBufferPtr);
I know it's unrelated, but erasing smart pointer types perhaps is not the
greatest idea. Could you please use concrete type instead of auto while you're
on this?
> region.cpp:61
> {
> -for (const auto &rect : region.rects()) {
> -installRegion(rect);
> +for (auto it = region.begin(); it != region.end(); ++it) {
> +installRegion(*it);
Heh, you could make it a bit simpler:
for (const QRect &rect : region) {
...
}
> surface_interface_p.h:26-27
> // Qt
> #include
> +#include
> // Wayland
Keep includes sorted please.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D24014
To: dfaure, davidedmundson, zzag
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24014: [KWayland] Port away from deprecated API in Qt 5.14
dfaure created this revision. dfaure added reviewers: davidedmundson, zzag. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. TEST PLAN Builds REPOSITORY R127 KWayland BRANCH master REVISION DETAIL https://phabricator.kde.org/D24014 AFFECTED FILES autotests/client/test_wayland_surface.cpp src/client/region.cpp src/client/surface.cpp src/server/surface_interface_p.h src/tools/generator.cpp src/tools/testserver/testserver.cpp To: dfaure, davidedmundson, zzag Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
