-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112631/#review39706
-----------------------------------------------------------

Ship it!


As a bodge I think it's ok.

> It turns out we only get this bug because kpeople is using a separate account 
> manager 
> to the main contact list, which is wrong. I'm sure I've said many times about 
> how we 
> shouldn't put two account managers in the same application because we'll get 
> weird 
> problems. Clearly not enough times.

Originally the account manager was returned by PersonsModel and only one was 
used, but that created the circular dependency, so it was moved to the 
IMDataSourcePlugin, which was again originally used directly, but with the 
model merge into one (ContactsListModel), we lost it. Too late for proper fix 
now, so let's ship with a working bodge, especially since a plan for proper fix 
already exists.

- Martin Klapetek


On Sept. 10, 2013, 12:17 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112631/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2013, 12:17 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> https://bugs.kde.org/show_bug.cgi?id=324698
> 
> Having finished this patch I've realised this is totally the wrong solution 
> to a problem that shouldn't exist.
> 
> It turns out we only get this bug because kpeople is using a separate account 
> manager to the main contact list, which is wrong. I'm sure I've said many 
> times about how we shouldn't put two account managers in the same application 
> because we'll get weird problems. Clearly not enough times.
> 
> I have a plan to fix that properly with singleton returning standard 
> factories..but it's too late to do that in 0.7. There's no other public way 
> to share the account factory.
> 
> I think maybe we should ship this, and I'll remove it in 0.8? Thoughts?
> 
> 
> Diffs
> -----
> 
>   KTp/Widgets/add-contact-dialog.h c12834a 
>   KTp/Widgets/add-contact-dialog.cpp fb34eb5 
> 
> Diff: http://git.reviewboard.kde.org/r/112631/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to