D9690: Add contacts-reading plugin (Android side)

[Matthijs Tijink]

mtijink requested changes to this revision.
mtijink added a comment.
This revision now requires changes to proceed.


  The code generally looks good to me! I added a few comments.

INLINE COMMENTS

> ContactsHelper.java:238
> +
> +        // WARNING -- UNDOCUMENTED BEHAVIOR USED AHEAD
> +        // Android returns the vcards as a big flat string of vcards

Do the individual vcards extracted here contain the id? If so, you can extract that and sort on that.

> ContactsPlugin.java:110
> +    public boolean onCreate() {
> +        permissionExplanation = contactsPermissionExplanation;
> +

You can use the `R.string.[...]` constant directly here.

> ContactsPlugin.java:117
> +    /**
> +     * Since this plugin could leak sensitive information, probably best to leave disabled by default
> +     */

Comment does not correspond with the code.

> ContactsPlugin.java:156
> +
> +        // Build the timestamp line
> +        // Maybe one day this should be changed into the vcard-standard REV key

These comments do not apply to the lines immediately below? Maybe it should be moved a few lines down.

REPOSITORY
  R225 KDE Connect - Android application

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

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