<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/101633/">http://git.reviewboard.kde.org/r/101633/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 15th, 2011, 11:45 p.m., <b>Daniele Elmo Domenichelli</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/101633/diff/1/?file=24528#file24528line765" style="color: black; font-weight: bold; text-decoration: underline;">nepomuk-storage.cpp</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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void NepomukStorage::setContactAvatar(const QString &path,</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">765</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="n">m_contacts</span><span class="p">.</span><span class="n">contains</span><span class="p">(</span><span class="n">identifier</span><span class="p">));</span></pre></td>
</tr>
<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">766</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">m_contacts</span><span class="p">.</span><span class="n">contains</span><span class="p">(</span><span class="n">identifier</span><span class="p">))</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;">is this Q_ASSERT necessary?</pre>
</blockquote>
<p>On June 17th, 2011, 3:55 p.m., <b>George Goldberg</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;">Yes. If it gets triggered that's a bug, and the best way to make developers notice is a crash in debug mode :p</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;">I should have read the documentation of Q_ASSERT before commenting this :)</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 15th, 2011, 11:45 p.m., <b>Daniele Elmo Domenichelli</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/101633/diff/1/?file=24529#file24529line33" style="color: black; font-weight: bold; text-decoration: underline;">telepathy.trig</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; "></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">33</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> nrl:maxCardinality "1" .</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;">Another issue is that avatar can be changed, but just dropping this property means that it will be impossible to search for all the pictures that have been avatar for a certain account.
To fix this, we could have a generic avatar property and a currentAvatar subProperty, but perhaps this is useless, what do you think?</pre>
</blockquote>
<p>On June 17th, 2011, 3:59 p.m., <b>George Goldberg</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 don't think Nepomuk generally stores the history of any other property in this way, so I don't think it's appropriate to do that for an Avatar. However, I don't really know how Nepomuk takes care of that. Or does it even take care of history of properties?</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;">I don't think that it should store the history of the property, but the history of the file containing the avatar, in the same way as we will store information about the contact that sent us a file...
Therefore if the avatar is cached/saved somewhere, also the information that tell us that that file was an avatar for a person should be stored.
Another way to solve this could be to have the avatar (or avatarForIMAccount) property with maxcardinality 1 from nco:IMAccount to nie:DataObject (or in the opposite direction) AND another avatar property from pimo:person to nie:DataObject (or in the opposite direction), that could have cardinality > 1 since telepathy allows just one avatar per account, but there is no reason why a person should have just 1 avatar.</pre>
<br />
<p>- Daniele Elmo</p>
<br />
<p>On June 15th, 2011, 6:16 p.m., George Goldberg wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Telepathy.</div>
<div>By George Goldberg.</div>
<p style="color: grey;"><i>Updated June 15, 2011, 6:16 p.m.</i></p>
<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;">This patch reinstates support for Contact Avatars in the Telepathy Nepomuk Service.
Avatar support for local accounts looks trickier so will come in a subsequent patch.
I'd particularly like comments on the changes to the Telepathy ontology. The rationale behind adding the telepathy:avatar property to nco:IMAccount is that although we can use nco:photo, this can contain multiple photos of the contact and we need to be able to identify the one that is the avatar for this IM account. The reason for it not being upstream is that it seems like telepathy-level implementation detail to me (although I will of course discuss this with upstream in due course to see whether they think it might belong there). The result is you are meant to identify one particular nco:photo of the nco:personContact to be the telepathy:avatar.</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;">Unit tests (updated appropriately) all pass.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=270864">270864</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>abstract-storage.h <span style="color: grey">(7d1be9b96711500ddd2ab5b6d9aa8791e51bcea1)</span></li>
<li>account.h <span style="color: grey">(b57ebf881e1bd661bd8db069cc3f5147ce10b515)</span></li>
<li>account.cpp <span style="color: grey">(e0cfdf0931c75126339e18fb2458a93d510b6e28)</span></li>
<li>contact.h <span style="color: grey">(ccc482aa36c3d8e38cfa89608043f198ef8be674)</span></li>
<li>contact.cpp <span style="color: grey">(bc5a29100e380bbcb7ae48930813d19842b0a836)</span></li>
<li>controller.cpp <span style="color: grey">(f320326534c8b76ea73097c906d82c2b90a35f22)</span></li>
<li>nepomuk-storage.h <span style="color: grey">(216239371b0656dff61c4a189a709703094b7418)</span></li>
<li>nepomuk-storage.cpp <span style="color: grey">(7d154494b759e2b9bca95d086f164f1a71ab1095)</span></li>
<li>telepathy.trig <span style="color: grey">(d5a07d202185cc0d9aaa0fbd681cfd0202fa88c1)</span></li>
<li>tests/controller-test.h <span style="color: grey">(cd85a675d731b43c0960e0b5ba3fb02d8aacade7)</span></li>
<li>tests/storage-test.h <span style="color: grey">(ab5f05f2e5faf6b0710c2dfd9f8d3f21ae06d330)</span></li>
<li>tests/storage-test.cpp <span style="color: grey">(2ff9428f57f2515c60c9fe7fb24c46d9f6907b7d)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101633/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>