<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/109821/">http://git.reviewboard.kde.org/r/109821/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 8th, 2013, 3:46 p.m. UTC, <b>Aaron J. Seigo</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;">the only possible downside i can see at the moment is that it is not possible anymore to say which branch rename failed, etc. that does not really matter since the old code wasn't doing that either .. but it was possible with the individual jobs being returned. in future, if this becomes needed/desired, i suppose the signals in GitRunner could be augmented with additional information. in any case, this looks like an incremental improvement over the existing code as is.</pre>
</blockquote>
<p>On April 9th, 2013, 6:32 p.m. UTC, <b>Giorgos Tsiapaliokas</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;">if the signals become something foo(bool ok), then we are ok. No?
</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;">No, because you still don't know *what* changed (e.g. which branch was removed). This only matters if more than one action happens concurrently (e.g. removing several branches in a row). I don't think this is a blocking problem at the moment, however, it's just something to keep in mind, and this patch can go in as is.</pre>
<br />
<p>- Aaron J.</p>
<br />
<p>On April 2nd, 2013, 6:24 a.m. UTC, Giorgos Tsiapaliokas 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 Plasma.</div>
<div>By Giorgos Tsiapaliokas.</div>
<p style="color: grey;"><i>Updated April 2, 2013, 6:24 a.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 implements a proper KJob and uses the job as an async when
and where it makes sense.
My opinion about the patch(feel free to ignore it:)
I tried to make all the jobs async but in some cases it was an overhead
and the code was looking bad, so I implemented the execSynchrousnly in order
to emphasize that the job is sync and to have a better result.</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;">I haven't found any regressions.</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>plasmate/savesystem/timeline.cpp <span style="color: grey">(4293dae)</span></li>
<li>plasmate/savesystem/timeline.h <span style="color: grey">(95e7e97)</span></li>
<li>plasmate/savesystem/gitrunner.cpp <span style="color: grey">(7520239)</span></li>
<li>plasmate/savesystem/gitrunner.h <span style="color: grey">(dfacc5b)</span></li>
<li>plasmate/savesystem/dvcsjob.cpp <span style="color: grey">(6f83307)</span></li>
<li>plasmate/savesystem/dvcsjob.h <span style="color: grey">(38df371)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109821/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>