D23839: Port away from Qt's foreach

This revision was automatically updated to reflect the committed changes. Closed by commit R278:1081a6b284ba: Port away from Qts foreach (authored by kossebau). REPOSITORY R278 KWindowSystem CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23839?vs=65768=65782 REVISION DETAIL

D23839: Port away from Qt's foreach

zzag accepted this revision. zzag added a comment. This revision is now accepted and ready to land. I tend to leave only one comment about troubling issue/problem and expect that the author of a patch will address all other occurrences of the issue/problem. However, let's get this

D23839: Port away from Qt's foreach

kossebau added a comment. @zzag Actually, while you commented on that one loop only, the same would be valid also for other loops touched in the patch. So, do you want const ref with all of them? As you can see by the existing code, it also already used values, not const ref, surely also

D23839: Port away from Qt's foreach

kossebau added inline comments. INLINE COMMENTS > zzag wrote in kwindowsystem_threadtest.cpp:61 > I asked that because `for (const Type : collection) {` is more common. > I know that const ref doesn't have any advantages here. (Gah, phab ate this comment before, rewriting) Getting the item by

D23839: Port away from Qt's foreach

kossebau added a comment. Thanks for review, Albert & Vlad. BTW; still one foreach left in macOS branch of code, which I could not test-drive, so did not change (also touching internals that I could not quickly understand if there is a chance to conflicting container changes in the call

D23839: Port away from Qt's foreach

zzag added inline comments. INLINE COMMENTS > zzag wrote in kwindowsystem_threadtest.cpp:61 > I asked that because `for (const Type : collection) {` is more common. > I know that const ref doesn't have any advantages here. In either case, this change is good to go. However, it would be great

D23839: Port away from Qt's foreach

zzag added inline comments. INLINE COMMENTS > kossebau wrote in kwindowsystem_threadtest.cpp:61 > Would a const ref make sense here, given the type nature of WId which boils > down to a integer matching the byte size of a pointer? I asked that because `for (const Type : collection) {` is more

D23839: Port away from Qt's foreach

kossebau added a comment. In D23839#528788 , @zzag wrote: > Did you use a script to create this patch? Nope, manually done. INLINE COMMENTS > zzag wrote in kwindowsystem_threadtest.cpp:61 > Capture the value by const ref please. Would

D23839: Port away from Qt's foreach

zzag added a comment. Did you use a script to create this patch? INLINE COMMENTS > kwindowsystem_threadtest.cpp:61 > const QList windows = KWindowSystem::stackingOrder(); > -foreach (auto wid, windows) { > +for (auto wid : windows) { > KWindowInfo

D23839: Port away from Qt's foreach

aacid added a comment. seems good to me, but let someone of the people that actually know the code to the +2 REPOSITORY R278 KWindowSystem REVISION DETAIL https://phabricator.kde.org/D23839 To: kossebau, #kwin, zzag Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham,

D23839: Port away from Qt's foreach

kossebau created this revision. kossebau added reviewers: KWin, zzag. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. kossebau requested review of this revision. REPOSITORY R278 KWindowSystem BRANCH portfromforeach REVISION DETAIL