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

[Aleix Pol Gonzalez]

apol requested changes to this revision.
apol added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> message.h:33
> +    // TYPE field values from Android
> +    enum types
> +    {

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.

> message.h:59
> +     */
> +    QString m_body;
> +

const? and the m_address too?

> telephonyplugin.cpp:110
> +        const QString messageBody = message.getBody();
> +        const QString contactName = "";
> +        const QString phoneNumber = message.getAddress();

Shouldn't initialize if it's empty.

> telephonyplugin.cpp:113
> +        QDBusReply<bool> reply = m_telepathyInterface.call(QStringLiteral("sendMessage"), phoneNumber, contactName, messageBody);
> +        if (reply) {
> +            return true;

`return reply`?
Actually nobody seems to be reading the result of `forwardToTelepathy`, you can probablty just omit returning anything and call asynchronously.

> telephonyplugin.cpp:125
> +{
> +    auto messages = np.get<QVariantList>("messages");
> +

const

> telephonyplugin.cpp:127
> +
> +    for (QVariant body : messages)
> +    {

const QVariant &

> telephonyplugin.h:85
> +     */
> +    Q_SCRIPTABLE void sendAllConversationsRequest();
> +

requestAllConversations()?

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, ahmedbesbes, 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/20180418/67c40526/attachment.html>