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