Review Request: Add a class providing a GlobalContact list throughout all accounts

David Edmundson kde at davidedmundson.co.uk
Mon Sep 10 12:58:25 UTC 2012



> On Sept. 10, 2012, 10:22 a.m., Daniele Elmo Domenichelli wrote:
> > KTp/global-contact-manager.h, line 31
> > <http://git.reviewboard.kde.org/r/106404/diff/1/?file=84548#file84548line31>
> >
> >     A constructor that takes a Tp::AccountManagerPtr could be useful? Or perhaps even a constructor that takes the same arguments as  Tp::AccountManager::create?
> 
> David Edmundson wrote:
>     That doesn't really work from experience.
>     AccountManager doesn't have a signal "I'm ready now", and becomeReady should only be called once. 
>     
>     By putting it in the ctor people don't create member variables till midway through the program and there's null pointers everywhere and it's horrible.
>     
>     Therefore this is based on what GlobalPresence and AccountsModel changed to; just doing nothing (and in particular not crashing) until setAccountManager is called with a ready accountManager.

Edit: Having looked through account-manager code this is all really stupid (both here and in GlobalPresence.. and in AccountsModel)

AccountManager (looks to) emit newAccount for every existing account added when it becomes ready.. at which point there's no reason not to have the accountManager in the constructor everywhere.

This will simplify quite a few bits of code \o/.


- David


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


On Sept. 9, 2012, 4:55 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106404/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2012, 4:55 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Add a class providing a GlobalContact list throughout all accounts.
> 
> So far 3 people have managed to get this wrong when implementing this, including Rohan, Me, George G and Dario so it would be good to have one class that does it right, also I don't like duplicate code.
> 
> This is to fix Rohan's patch in the KDED module for monitoring account presences as well as removing all the duplciate code that will be in contact-request-handler. This code will also be useful for the nepomuk-service which also has an implementation of this.
> 
> This will also fix the contact-request-handler not being able to display which account a new contact request comes from.
> 
> 
> Diffs
> -----
> 
>   KTp/CMakeLists.txt fa0c741564e09c020fbe3d3e8b3d375fe9c4c3df 
>   KTp/global-contact-manager.h PRE-CREATION 
>   KTp/global-contact-manager.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106404/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


More information about the KDE-Telepathy mailing list