<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123184/">https://git.reviewboard.kde.org/r/123184/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 30th, 2015, 5:48 p.m. CEST, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If the host application is required to check for a property, then this should be documented as well.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm unsure about the use-case there though. Why do we need to filter the actions? If we just want chat, wouldn't it be better to directly request it to KTp?</p></pre>
</blockquote>
<p>On March 30th, 2015, 5:52 p.m. CEST, <b>Eike Hein</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm unsure about the use-case there though. Why do we need to filter the actions? If we just want chat, wouldn't it be better to directly request it to KTp?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We don't want frontends to be tied to KTp in the long run - we want other messengers (e.g. Konversation) to integrate with KPeople, and a programatic way to start a conversation using the user's preferred messenger (we already have a Default Apps setting for one), depending on presence.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Something like startTextChat(PersonData *data) would be much better to hide the implementation instead of relying on action list sorting to return the preferred TextChatAction first, but as KTp is the only available backend right now this is fine for use in Plasma 5.3, there's no need to rush the API design.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There was some email forth and back on this you were CC'd on btw.</p></pre>
</blockquote>
<p>On March 30th, 2015, 6:03 p.m. CEST, <b>Martin Klapetek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Clients are not required for checking the property, they can if they want/need to, as Eike explains. Adding more metadata to the actions only allows for better flexibility, this would also allow for example to group related actions together in say address book.</p></pre>
</blockquote>
<p>On March 30th, 2015, 6:11 p.m. CEST, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Still, the property is not documented.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We don't need to rush an API design, but we definitely need to find ways to move away from the current QWidgets-based approach.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Anyway, I guess this works, I'm happy to have it in and see how it develops.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also I've put it with PersonData because the plugins implementing the actions do not include actions.h but they always include persondata.h because it's being passed to it. I don't think that putting it into actions.h makes too much sense because that's client only, not for plugins which also need that enum (and the plugins don't link to KPeopleWidgets either). But if you think it should go there, I'll move it there.</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On March 30th, 2015, 5:03 p.m. CEST, Martin Klapetek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Telepathy and Eike Hein.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated March 30, 2015, 5:03 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kpeople
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Some clients need/want to filter actions based on types, this adds the actions enum with currently known actions. Plugins returning QList<QAction*> should then use this enum to set the appropriate type on the action (via setProperty).
I've put it to PersonData because all code dealing with actions currently need to include PersonData header and I'm not sure if it makes sense anywhere else.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/persondata.h <span style="color: grey">(c3f99a9)</span></li>
<li>src/widgets/actions.h <span style="color: grey">(2931ef8)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123184/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>