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

[David Edmundson]

> 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?

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.


> On Sept. 10, 2012, 10:22 a.m., Daniele Elmo Domenichelli wrote:
> > KTp/global-contact-manager.h, lines 13-21
> > <http://git.reviewboard.kde.org/r/106404/diff/1/?file=84548#file84548line13>
> >
> >     Perhaps it could derive from QPair<Tp::AccountPtr, Tp::ContactPtr> to get some operators for free?
> >     
> >     By the way since you are exporting this class and I believe that it could be used in several other places, is it worth to have it in a separate file? (Or perhaps we can just add the a pretty header for this class that include this file)

Yeah, I need to talk to Martin about what other plans are with contacts


- 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/79c5985b/attachment.html>