----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100968/#review2212 -----------------------------------------------------------
A few comments below. I would suggest using an AccountFactory (a Tp-Qt 0.5 features) when you create the accountManager, it simplifies the code quite a lot, and stops you retrieving data before it's ready by accident. presence.h <http://git.reviewboard.kde.org/r/100968/#comment1846> suggest onAccountAdded() for consistency up to you, as you do also call it like a conventional method as well. presence.cpp <http://git.reviewboard.kde.org/r/100968/#comment1847> If you're using and AccountSet of valid accounts you shouldn't be monitoring the accounts for accountValidityChanged, nor for accountManager fo newAccount / accountRemoved. An AccountSet is a lot more than just a simple typedef, you haven't made the full use of it. You should watch the AccountSet for accountAdded and accountRemoved, which is doing all 3 of the above for you whilst applying the filter. (at least that's how I understand it) (currently yours doesn't list invalid accounts at the start, but will list any newly added invalid accounts, this will fix that) presence.operations <http://git.reviewboard.kde.org/r/100968/#comment1845> should this not say setAvatar? presencesource.cpp <http://git.reviewboard.kde.org/r/100968/#comment1843> qDebug -> kDebug presencesource.cpp <http://git.reviewboard.kde.org/r/100968/#comment1844> qDebug -> kDebug - David On March 29, 2011, 12:17 p.m., Dario Freddi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100968/ > ----------------------------------------------------------- > > (Updated March 29, 2011, 12:17 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > This patch ports the data engine to Telepathy Qt4 0.5.x, and adds new Service > operations for setting avatars and nicknames. It also enhances data retrieval > and fixes some small bugs here and there. > > > Diffs > ----- > > CMakeLists.txt 409c6135e53ecee63f02a756a9c0d8c5c399fe4e > plasma-dataengine-presence.desktop 7a850af570113882b1d8c3509e3c260a2fc77353 > presence.h eb5d6dac20a9e7469508d4b87bb7fd02a5624c24 > presence.cpp 179c8055bbf29bd17eb80faf8fd14a8549a453a9 > presence.operations 0aed15758af9cab6780bd8b31822ce991d045983 > presenceservice.cpp d7a9a64d3b04acbb5ae19c9b7194c2f47efae74c > presencesource.h 586b70f258d7b9bd0bc239aba25b599a038cfd4a > presencesource.cpp fdab2c400e8833844c7feed71d77927acd9160cf > setavatarjob.h PRE-CREATION > setavatarjob.cpp PRE-CREATION > setnicknamejob.h PRE-CREATION > setnicknamejob.cpp PRE-CREATION > setrequestedpresencejob.h 40c2ca1f5abff5d82c49f9ac1019b162e9249128 > setrequestedpresencejob.cpp 0a77a48941ab41cebe59b6646ba5bbde796ad7a4 > > Diff: http://git.reviewboard.kde.org/r/100968/diff > > > Testing > ------- > > Engine explorer works flawlessly > > > Thanks, > > Dario > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
