D9691: Add contacts-reading plugin (KDE side)

[Matthijs Tijink]

mtijink added inline comments.

INLINE COMMENTS

> contactsplugin.cpp:70
> +    {
> +        return this->handleResponseUIDsTimestamps(np);
> +    } else if (np.type() == PACKET_TYPE_CONTACTS_RESPONSE_VCARDS)

It's not really common in C++ to write `this->`.

> contactsplugin.h:69
> + */
> +#define PACKET_TYPE_CONTACTS_RESPONSE_VCARDS QStringLiteral("kdeconnect.contacts.response_vcards")
> +

You can get the uid's by calling `map.keys()`, so the separate list is not needed.

> contactsplugin.h:75
> + */
> +Q_GLOBAL_STATIC_WITH_ARGS(QString, vcardsLocation, (QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) + ("/kpeoplevcard")))
> +

Wouldn't `kdeconnect/vcard` make more sense?

> contactsplugin.h:122
> +
> +protected:
> +

Why protected? We're not going to inherit from this, right?

> kdeconnect_contacts.json:13
> +        "Description[x-test]": "xxSynchronize Contacts Between the Desktop and the Connected Devicexx",
> +        "EnabledByDefault": true,
> +        "Icon": "dialog-ok",

How will the contact integrate with the desktop? E.g. will I see the contacts in KAddressBook? Or can I sent people e-mails using this?

If the contacts conflict with what the user has already set up, I'd disable it by default. Otherwise, this is fine.

REPOSITORY
  R224 KDE Connect

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

To: sredman, #kde_connect, apol
Cc: mtijink, #kde_connect, apol, adeen-s, SemperPeritus, ahmedbesbes, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, ach
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180329/64443287/attachment.html>