D11854: (WIP) Upgrade Telephony plugin to read SMS history (KDE side)

Simon Redman noreply at phabricator.kde.org
Tue May 15 22:24:04 UTC 2018


sredman marked 10 inline comments as done.
sredman added inline comments.

INLINE COMMENTS

> apol wrote in message.h:33
> Usually enums start with a capital letter and values are camelCased. Also no need to specify the value for each.
> Specify it a Q_ENUM(types) too, it's better for debugging at least.

I have specified the values since they come from Android's values for the same field, just to be clear that they are the same (even though I know they would automatically be). Do you think this is a good idea?
Done on the rest of the suggestions. Thanks!

> nicolasfella wrote in telephonyplugin.cpp:60
> Else if?

At this revision I hadn't figured out what to do with old-style packets. I have decided to support them fully (meaning old Android apps would continue to function as they currently do) and return at the end of the if block. Does this work for you?

> nicolasfella wrote in telephonyplugin.cpp:108
> Use new syntax if possible

The messageReceived method being used as the sender of this connection is on the other side of the DBus interface (telepathy-kdeconnect/connection.h line 55). I don't know how or if the new syntax can be used in this case

REPOSITORY
  R224 KDE Connect

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

To: sredman, #kde_connect, nicolasfella, apol
Cc: apol, nicolasfella, #kde_connect, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, ndavis, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, ach
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180515/a330f5fc/attachment.html>


More information about the KDEConnect mailing list