<table><tr><td style="">sredman added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D9690" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D9690#189003" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D9690#189003</a>, <a href="https://phabricator.kde.org/p/mtijink/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@mtijink</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>Looks promising (didn't test it though), so I added a few initial comments. It's best if you think what <em>you</em> want to use it for. Otherwise you're implementing something which may never be used, or is unsuitable for an use case.</p></div>
</blockquote>
<p>Thanks for the feedback!</p>
<p>For the in-line comments I didn't reply to, thank you for the good suggestions! I will implement them.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9690#inline-44476" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mtijink</span> wrote in <span style="color: #4b4d51; font-weight: bold;">ContactsPlugin.java:106</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Two things: wouldn't it make more sense to keep the structure of the Android contacts (i.e. multiple numbers for one contact, so only send the name once)?</p>
<p style="padding: 0; margin: 8px;">And the index is never used (and does not uniquely correspond to a single contact), so you don't need to send it.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I did it this way because I couldn't figure out how to use NetworkPackage to just send a pile of things and get them out on the other side, so I assign them unique (sequential) integers here, and the pull them out using the same scheme on the other side...</p>
<p style="padding: 0; margin: 8px;">However, I just had a much better idea. One problem I haven't been able to solve is how to distinguish between two contacts with the same name, but Android has already solved this problem by having unique keys assigned to every contact. So my new thought is to ship the list of IDs, then have each contact in the package stored with that ID as the key. I will have a new version of this plugin up soon!</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R225 KDE Connect - Android application</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D9690" rel="noreferrer">https://phabricator.kde.org/D9690</a></div></div><br /><div><strong>To: </strong>sredman, kdeconnect<br /><strong>Cc: </strong>mtijink<br /></div>