D7446: [Places panel] Revamp the Recently Saved section
Elvis Angelaccio
noreply at phabricator.kde.org
Fri Sep 13 13:54:18 BST 2019
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190913/f0029a22/attachment.html>
More information about the Kde-frameworks-devel
mailing list