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

Simon Redman noreply at phabricator.kde.org
Thu Sep 20 17:41:14 BST 2018


sredman added a reviewer: KDE Connect.
sredman added a comment.


  In D15608#328757 <https://phabricator.kde.org/D15608#328757>, @apol wrote:
  
  > LGTM overall, I'm a bit concerned this is getting a bit too complex.
  
  
  I agree. Can you see anywhere where we can make it better, though? This patch actually removes one call/response on the dbus interface by directly providing the message contents instead of requesting them with the threadID, so nice from that point of view.
  
  I will look into your other comments as soon as possible

INLINE COMMENTS

> conversationlistmodel.cpp:63-66
> +    if (deviceId == "") {
> +        // When the device is first connecting we get junk
> +        return;
> +    }

What do you think about this being above the checks to delete m_conversationsInterface? That way, when the device is disconnected, you can still browse messages locally cached in the conversationDbusInterface.

> apol wrote in conversationlistmodel.cpp:98
> leave empty?

I left this here so I could put a breakpoint on it and decide what to do with the incoming data 😬

Working on filling out this method is my next to-do to solve bug 398818 <https://bugs.kde.org/show_bug.cgi?id=398818>

REPOSITORY
  R224 KDE Connect

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

To: sredman, #kde_connect
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/d237b4a5/attachment.html>


More information about the KDEConnect mailing list