D24350: [src/kpac/*] replace deprecated foreach with range for
This revision was automatically updated to reflect the committed changes. Closed by commit R241:103e13c2765e: [src/kpac/*] replace deprecated foreach with range for (authored by ahmadsamir). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24350?vs=68395=68419 REVISION DETAIL https://phabricator.kde.org/D24350 AFFECTED FILES src/kpac/script.cpp To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24350: [src/kpac/*] replace deprecated foreach with range for
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH ahmad/foreach-kpac (branched from master) REVISION DETAIL https://phabricator.kde.org/D24350 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24350: [src/kpac/*] replace deprecated foreach with range for
ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in script.cpp:157 > Why not just change this one to return `const &` and then use it in all > range-fors? > > [note that returning a const ref is bad in public API, but this is an > internal method so we can always change it again if needed] Because I didn't pay attention to public v.s. private API :) Fixing. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24350 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24350: [src/kpac/*] replace deprecated foreach with range for
ahmadsamir updated this revision to Diff 68395. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. In private API we can have a method return a const &, as it can be changed later without side-effects. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24350?vs=67152=68395 BRANCH ahmad/foreach-kpac (branched from master) REVISION DETAIL https://phabricator.kde.org/D24350 AFFECTED FILES src/kpac/script.cpp To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24350: [src/kpac/*] replace deprecated foreach with range for
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
sorry, missed this one. thanks for the ping.
INLINE COMMENTS
> script.cpp:157
>
> QList addresses() const
> {
Why not just change this one to return `const &` and then use it in all
range-fors?
[note that returning a const ref is bad in public API, but this is an internal
method so we can always change it again if needed]
> script.cpp:162
>
> +const QList () const
> +{
it's just weird to have two methods that do the same thing, with a different
name.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D24350
To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24350: [src/kpac/*] replace deprecated foreach with range for
ahmadsamir added a comment. Ping... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24350 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24350: [src/kpac/*] replace deprecated foreach with range for
ahmadsamir created this revision. ahmadsamir added a reviewer: dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY Add convience function that returns const QList&, since that list is iterated over in several places in the code. TEST PLAN make && ctest REPOSITORY R241 KIO BRANCH ahmad/foreach-kpac (branched from master) REVISION DETAIL https://phabricator.kde.org/D24350 AFFECTED FILES src/kpac/script.cpp To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
