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

Aleix Pol Gonzalez noreply at phabricator.kde.org
Wed May 16 15:28:52 UTC 2018


apol added inline comments.

INLINE COMMENTS

> message.h:28
> +    Q_OBJECT
> +    Q_CLASSINFO("D-Bus Interface", "org.kde.kdeconnect.device.telephony.messages")
> +    Q_PROPERTY(QString body READ getBody)

Mark `CONSTANT` if it's not going to change.

> message.h:40
> +    {
> +        messageTypeAll = 0,
> +        messageTypeInbox = 1,

Enums usually start with capital letter: MessageTypeAll

> message.h:58
> +
> +    QString getBody() const { return m_body; }
> +    QString getAddress() const { return m_address; }

getters in Qt usually don't have a get prefix: getBody() -> body()

> telephonyplugin.cpp:50
>      const QByteArray phoneThumbnail = QByteArray::fromBase64(np.get<QByteArray>(QStringLiteral("phoneThumbnail"), ""));
> +    const QString messageBody = np.get<QString>(QStringLiteral("messageBody"),QLatin1String(""));
>  

Instead of passing "" or QLatin1String(""), pass `QString()` or even `{}`.

> telephonyplugin.cpp:53
>      // In case telepathy can handle the message, don't do anything else
>      if (event == QLatin1String("sms") && m_telepathyInterface.isValid()) {
> +        // Telepathy has already been tried (in receivePacket)

Shouldn't this be `!m_telepathyInterface.isValid()`?

REPOSITORY
  R224 KDE Connect

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

To: sredman, #kde_connect, nicolasfella, apol
Cc: andyholmes, 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/20180516/1284fa3c/attachment.html>


More information about the KDEConnect mailing list