dfaure closed this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D24160
To: dfaure, bruns, kossebau
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
kossebau accepted this revision.
kossebau added a comment.
This revision is now accepted and ready to land.
Code changes look all fine to me. Only looked at code here with eyes (tired
and fermentation products influenced ;) ), not built, run or tested though.
REPOSITORY
R241 KIO
BRANCH
2
dfaure updated this revision to Diff 66991.
dfaure marked 4 inline comments as done.
dfaure added a comment.
Make changes suggested by Friedrich's code review (thanks!)
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D24160?vs=5&id=66991
BRANCH
2019_09_mod
dfaure marked 18 inline comments as done.
dfaure added inline comments.
INLINE COMMENTS
> kossebau wrote in kmountpoint.cpp:344
> Why change away from auto?
Consistency with all other range for loops in kio, and
https://wiki.qt.io/Coding_Conventions#auto_Keyword
> kossebau wrote in kmountpoint
kossebau added a comment.
Not tested the changes myself, only looked at the code here.
Looks good to me in general, modulo the comments I made.
When it comes to `it`, my code reading expectations are that it is a real
iterator, same like `i` is an integer index. The current patch propos
kossebau added a comment.
Will see to give this some review later this week (if still needed). Would
agree the "modern" code is more easy to grasp by humans and thus better to
maintain, so +1 for principle.
All override changes please as separate commit though, unrelated to commit
messa
dfaure created this revision.
dfaure added reviewers: bruns, kossebau.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dfaure requested review of this revision.
REVISION SUMMARY
This was done using clang-tidy's modernize-loop-convert, then
clazy (for qAsCon
