<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/114559/">http://git.reviewboard.kde.org/r/114559/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 20th, 2013, 4:49 p.m. YEKT, <b>Alexandr Akulich</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/114559/diff/1/?file=226653#file226653line49" style="color: black; font-weight: bold; text-decoration: underline;">src/basepersonsdatasource.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">49</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="n">QString</span> <span class="n">sourceId</span><span class="p">()</span> <span class="k">const</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So, one plugin can provide only one source? I'm ok for that, but as it ABI changes, we should think again - what's if plugin support several simular sources via one codebase? Though, such code can be extracted to library.</pre>
</blockquote>
<p>On December 20th, 2013, 7:18 p.m. YEKT, <b>David Edmundson</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;">I can't see it being a problem - it's a name to match a hidden identifier to a plugin so we know which plugin to query to get data.
Inside a plugin we can do multiple things, like Akonadi abstracts all sorts of crazy real sources - but to kpeople it's all just "akonadi"
Maybe source is a bad name?</pre>
</blockquote>
<p>On December 20th, 2013, 8:39 p.m. YEKT, <b>Alexandr Akulich</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;">May be my understanding is not correct. I thinking that it is like "protocol" or "schema", so it is possible that several different plugins can provide same "sourceId" (sourceId is string like "skype" or "akonadi" or may be "jabber" and so on), so user can select prefered plugin/backend. At same time, one plugin (somewhat like gabble) may provide several sources.</pre>
</blockquote>
<p>On December 21st, 2013, 2:26 a.m. YEKT, <b>David Edmundson</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;">It's so that the PersonData knows which plugin to query to get the data. So jabber + msn + icq etc, will all be "ktp". If Kopete gets kpeople support the source would be "kopete" regardless of the underlying real source.
Each plugin then only has to keep the local ID unique within it's own protocol. I.e ktp can use a mesh the accounts+protocols,etc, akonadi can just keep their silly integer.
I really don't want to allow two plugins to provide the same source (i.e two people having "jabber" or "facebook") as it would end up in a horrible conflicting mess.</pre>
</blockquote>
<p>On December 21st, 2013, 6:20 a.m. YEKT, <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;">I think better naming that method might help - sourcePluginId()?
As it is pure virtual method and every extending class will have to implement, add comment what it needs to do.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">+1 for sourcePluginId().</pre>
<br />
<p>- Alexandr</p>
<br />
<p>On December 20th, 2013, 6:27 a.m. YEKT, David Edmundson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Telepathy.</div>
<div>By David Edmundson.</div>
<p style="color: grey;"><i>Updated Dec. 20, 2013, 6:27 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
libkpeople
</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;">Only try loading data from the relevant data source
i.e only query akonadi for contacts starting with akonadi://
Thoughts:
- should this be a pure virtual method in the data source or a property in the .desktop file?
- the parsing of the IDs to get the sourceId from the url-looking string is a bit rubbish, in a future patch I'll tidy this up with an ID class that doesn't have this horrible string parsing.
Note this is an ABI break. </pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Opened PersonViewer</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/basepersonsdatasource.h <span style="color: grey">(12f698e)</span></li>
<li>src/persondata.cpp <span style="color: grey">(277fa6b)</span></li>
<li>src/personpluginmanager.h <span style="color: grey">(adc7b9d)</span></li>
<li>src/personpluginmanager.cpp <span style="color: grey">(e433bb5)</span></li>
<li>src/plugins/akonadi/akonadidatasource.h <span style="color: grey">(ef5c602)</span></li>
<li>src/plugins/akonadi/akonadidatasource.cpp <span style="color: grey">(b640d00)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/114559/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>