D15608: [Desktop] Prevent SMS app from requesting entire contents of every conversation

Aleix Pol Gonzalez noreply at phabricator.kde.org
Thu Sep 20 10:02:57 BST 2018


apol added a comment.


  LGTM overall, I'm a bit concerned this is getting a bit too complex.

INLINE COMMENTS

> conversationsdbusinterface.cpp:53
> +    {
> +        const QVariantMap& message = (*conversation.cbegin()).toVariant();
> +        toReturn.append(QVariant(message));

Looks like it's casting twice? How about `toReturn += conversation.cbegin()->toVariant()`?
Also we probably should make sure it's not empty.

> conversationsdbusinterface.cpp:79
> +    // TODO: BUG: What if the caller asked for start != 0?
>      for(auto it = messagesList.crbegin(); it != messagesList.crend(); ++it)
>      {

Should probably offset it by start?

> conversationlistmodel.cpp:98
> +{
> +    return;
>  }

leave empty?

REPOSITORY
  R224 KDE Connect

REVISION DETAIL
  https://phabricator.kde.org/D15608

To: sredman
Cc: apol, kdeconnect, wistak, dvalencia, rmenezes, julioc, Leptopoda, timothyc, jdvr, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, ndavis, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, tctara
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180920/96bb08ca/attachment.html>


More information about the KDEConnect mailing list