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