----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115328/#review48369 -----------------------------------------------------------
contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34206> You can delete this. contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34198> and we need an index on the first two columns contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34201> Just a reminder for me. We must update the rest of the kded module to use KTp::accountManager otherwise we will have 2. Which means twice as much DBus traffic. contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34199> I can imagine this being a problem. .bind automatically escapes for us. which means we end up saying WHERE accountID NOT IN ('myAccount1,myAccount2') instead of ('myAccount1','myAccount2') contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34204> I'm surprised this works. At no point do we need to call connection->becomeReady(FeatureRoster); at which point it's not trying to load the whole roster (contact list). We have this off by default in the ConnectionFactory to make loading faster. contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34200> coding style {} contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34202> we need to use onConnectionChanged On the account DBus object we have 2 parameters; connectionStatus and a path to the connection DBus object. isOnline is a wrapper round the connectionState. At the time this happens, we have the path to the new DBus connection object, but we have not finished loading our Tp::Connection object yet. This will happen nanoseconds afterwards. Tp::Account::connection() is only set once Tp::Connection has loaded. Basically you end up in a race condition. Your code does exit out gracefully in the error checks, but we will miss a lot of updates. contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34203> why have an if on the connect()? contact-cache.cpp <https://git.reviewboard.kde.org/r/115328/#comment34205> Is this actually needed? My KTp::Account sets a property on the connection object. At the time a contactManager state changes you will always have a connection object. at which point we can do if (contactManager->connection()) { const QString accountPath = contactManager->connection->property("accountUID").toString(); Tp::AccountPtr account = KTp::accountManager()->accountForPath(accountPath)); } The whole connection->account mapping always results in a bit of a hack, I don't want to have two hacks. - David Edmundson On Jan. 27, 2014, 11:52 a.m., Alexandr Akulich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115328/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2014, 11:52 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-kded-module > > > Description > ------- > > Was: https://git.reviewboard.kde.org/r/115060/ > > Draft version. Not proposed to be merged as is. > > TODO: > * Fix code > * Add groups info > * Performance testing. > > > Diffs > ----- > > telepathy-module.cpp a754283 > contactnotify.cpp 314d48a > contact-cache.h PRE-CREATION > contact-cache.cpp PRE-CREATION > CMakeLists.txt a31245b > > Diff: https://git.reviewboard.kde.org/r/115328/diff/ > > > Testing > ------- > > Tested accounts contact refreshing in follow action: > 1 Does connect via Telepathy. > 2 Checked that contacts added to db (via sqlite db viewer). > 3 Disconnected in telepathy. > 4 Connect via another IM software. > 5 Removed few contacts. > 6 Does connect via Telepathy. > 7 Checked that there is only actual contacts in db (via sqlite db viewer). > > > Thanks, > > Alexandr Akulich > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
