Review Request 115943: Fixes duplication of groups when renaming it.

David Edmundson david at davidedmundson.co.uk
Sat Mar 1 14:38:34 UTC 2014



> On Feb. 22, 2014, 9:21 p.m., David Edmundson wrote:
> > 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?
> >
> 
> mayank jha wrote:
>     Can we add a delay in between the Add and Remove, making it even slower? That will most certainly remove the bug.
> 
> mayank jha wrote:
>     I guess waiting for adding to complete would be as costly as adding a delay in between add and remove, is it not ?
> 
> David Edmundson wrote:
>     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.
>     
>
> 
> mayank jha wrote:
>     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)?

>I am also not quite sure about the exact cause of the bug,

Then how will you know how to fix it?


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115943/#review50557
-----------------------------------------------------------


On Feb. 21, 2014, 8:24 p.m., mayank jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115943/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2014, 8:24 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Bugs: 329904
>     http://bugs.kde.org/show_bug.cgi?id=329904
> 
> 
> Repository: ktp-contact-list
> 
> 
> Description
> -------
> 
> 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!
> 
> 
> Diffs
> -----
> 
>   context-menu.cpp 00795d7 
> 
> Diff: https://git.reviewboard.kde.org/r/115943/diff/
> 
> 
> Testing
> -------
> 
> Testing Done, it works fine!
> 
> 
> Thanks,
> 
> mayank jha
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140301/c9d8893e/attachment.html>


More information about the KDE-Telepathy mailing list