<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/113235/">http://git.reviewboard.kde.org/r/113235/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 18th, 2013, 11:28 a.m. UTC, <b>Daniele E. Domenichelli</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 looks ok to me.
Anyway 3 questions:
1) You start a KJob for each resource, perhaps starting just one KJob to delete all the resources would be more efficient since it already takes some time the first time you start the contact list, but I don't know if there is difference from nepomuk side.
2) You start the KJobs syncronously, maybe one single async kjob deleting all the resources would be better.
3) You never check the result values of the kjobs... I'm not sure if this might be useful, but maybe for debugging purpose it's worth printing something.
</pre>
</blockquote>
<p>On October 18th, 2013, 2:05 p.m. UTC, <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;">> 2) You start the KJobs syncronously, maybe one single async kjob deleting all the resources would be better.
I thought about it too, but you should not proceed with any other stuff unless the database is clean, so it's ok it's sync imho</pre>
</blockquote>
<p>On October 18th, 2013, 2:30 p.m. UTC, <b>Daniele E. Domenichelli</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 should not do anything until the job finishes, but the problem, according to http://api.kde.org/4.x-api/kdelibs-apidocs/kdecore/html/classKJob.html is that if the KJob::exec() caller is not reentrant, it is bad... And since I have no idea about what is calling it and if it will change in the future, I'd go for the safe way.</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;">If KJob::Exec() was dangerous we would have a tonne of problems throughout all KDE code.
We do all the execs() from a single thread so I can't imagine any problems.</pre>
<br />
<p>- David</p>
<br />
<p>On October 17th, 2013, 9:52 p.m. UTC, 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 Oct. 17, 2013, 9:52 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ktp-common-internals
</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;">Wipe all KTp contacts when we release
This is needed if we ship https://git.reviewboard.kde.org/r/112970/ for 0.7.0.
</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>kpeople/nepomuk-feeder/controller.h <span style="color: grey">(b358c8e)</span></li>
<li>kpeople/nepomuk-feeder/controller.cpp <span style="color: grey">(fa37b7e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113235/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>