----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106404/#review18785 -----------------------------------------------------------
Just a quick review of the API... This looks like a very useful addition, perhaps it's even worth to have it in tp-qt KTp/global-contact-manager.h <http://git.reviewboard.kde.org/r/106404/#comment14895> 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) KTp/global-contact-manager.h <http://git.reviewboard.kde.org/r/106404/#comment14900> A constructor that takes a Tp::AccountManagerPtr could be useful? Or perhaps even a constructor that takes the same arguments as Tp::AccountManager::create? KTp/global-contact-manager.h <http://git.reviewboard.kde.org/r/106404/#comment14896> white spaces KTp/global-contact-manager.h <http://git.reviewboard.kde.org/r/106404/#comment14899> To avoid ABI problems in the future I think that these should be in the private class and slots declared using Q_PRIVATE_SLOT KTp/global-contact-manager.h <http://git.reviewboard.kde.org/r/106404/#comment14897> white spaces - Daniele Elmo Domenichelli 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 > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
