<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="https://git.reviewboard.kde.org/r/115943/">https://git.reviewboard.kde.org/r/115943/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 22nd, 2014, 9:21 p.m. UTC, <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'm not sure this will definitely fix the bug.
We've reordered it from
Add
Remove
Add
Remove
...
To
Add
Add
Remove
Remove
...
Which is undoubtedly better, but if it was a race condition, we'll still have a race condition; albeit one that is now marginally less likely to happen. For the case of one contact, the code path is identical.
Should we wait for add to complete before calling remove?
</pre>
</blockquote>
<p>On February 23rd, 2014, 11:54 a.m. UTC, <b>mayank jha</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;">Can we add a delay in between the Add and Remove, making it even slower? That will most certainly remove the bug. </pre>
</blockquote>
<p>On February 24th, 2014, 9:51 a.m. UTC, <b>mayank jha</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 guess waiting for adding to complete would be as costly as adding a delay in between add and remove, is it not ?</pre>
</blockquote>
<p>On February 24th, 2014, 11:29 a.m. UTC, <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;">How long will you make the delay. We can't know.
Also I don't fully understand what makes the current code broken. DBus calls are always processed in order, so why would this result in the group still existing in the contact list? Do you know?
What I meant by wait for it finish is to it asynchronously. i.e connect to the finished signal of the PendingOperation and do the removing there.
This means we are not blocking our application whilst we wait for the backend to move contacts.
</pre>
</blockquote>
<p>On February 24th, 2014, 7:16 p.m. UTC, <b>mayank jha</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 am also not quite sure about the exact cause of the bug, but I think asynchronous waiting would be the safer approach. But would it be a one-by-one asynchronous check or:
Add (all)
emit complete
Remove (all)?</pre>
</blockquote>
<p>On March 1st, 2014, 2:38 p.m. UTC, <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 am also not quite sure about the exact cause of the bug,
Then how will you know how to fix it?</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;">I am trying to ascertain the cause of the bug. I tested the patch rigourously multiple times, and the cause seems to be the scheduling of operations.</pre>
<br />
<p>- mayank</p>
<br />
<p>On February 21st, 2014, 8:24 p.m. UTC, mayank jha wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 mayank jha.</div>
<p style="color: grey;"><i>Updated Feb. 21, 2014, 8:24 p.m.</i></p>
<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=329904">329904</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ktp-contact-list
</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;">This patch fixes, the group duplication upon renaming the group. This is probably due to the scheduling of the operations of adding and removing contacts from groups, so I separated the two, and it works fine!</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;">Testing Done, it works fine!</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>context-menu.cpp <span style="color: grey">(00795d7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115943/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>