elvisangelaccio added a comment.
LGTM besides the inline nitpicks.
INLINE COMMENTS
> kfileplacesmodeltest.cpp:686
> KBookmark device = root.first(); // The device we'll move is the 6th
> bookmark
> - for (int i = 0; i < 5; i++) {
> + int stop = m_hasRecentlyUsedKio ? 7 : 5;
> + for (int i = 0; i < stop; i++) {
`const`, and maybe a more descriptive name (`count` or similar?)
> kfileplacesmodeltest.cpp:967
> // places
> - QTest::newRow("Places - Home") << m_places->index(0, 0)
> + int idx = 0;
> + QTest::newRow("Places - Home") << m_places->index(idx++, 0)
I'd avoid cryptic names. If we can't use `index`, maybe just `i`?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D7446
To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel,
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio,
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n,
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh,
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
