There are quite a few even with the current state of the mix-and-match qt5 megabranch for bug 43222. The notes can be found most easily by case insensitive grep for todo|fixme. These are things which we haven't been able to mark with the DEPRECATED macro, as that's only applicable to methods and types to be removed, or the runtime signal deprecation system.
Branch at URL attempts to fix this. I searched for all TODO/FIXMEs there and fixed those that requires fixing during the API/ABI break. The ones remaining can be fixed later if/when appropriate. Also all deprecated signals/methods were removed. Will update NEWS when merging this.
(In reply to comment #1) > Branch at URL attempts to fix this. > > I searched for all TODO/FIXMEs there and fixed those that requires fixing > during the API/ABI break. The ones remaining can be fixed later if/when > appropriate. So it appears. > Also all deprecated signals/methods were removed. > Will update NEWS when merging this. Good! Some observations AbstractClient::AbstractClient() + : mPriv(new Private()) { } I see no corresponding delete, so this is leaked. AbstractClient dtor? > Channel: Add includeSelfContact param to groupSelfContact and groupLocal/RemotePendingContacts. Commit message clearly refers wrongly to groupSelfContact(). Also, docs missing for the includeSelfContact params. + if (!includeSelfContact) { + return mPriv->groupRemotePendingContacts.values().toSet() - Contacts() << groupSelfContact(); + } return mPriv->groupRemotePendingContacts.values().toSet(); Something like Contacts c = mPriv->whicheverContactsYoureReturning.values().toSet(); if (!includeSelfContact) { c.remove(groupSelfContact()); } return c; would be both cleaner (no duplicate mention of the source container) and faster (no singleton set created and trivially iterated for the self contact). Otherwise looks good to me. I didn't look in Jeremy's big patches in detail, just an overview; I trust you've already done that when inheriting the work. Adding dependency because the tp-qt (instead of tp-qt4) naming here depends on the qt5 branch.
(In reply to comment #2) > AbstractClient::AbstractClient() > + : mPriv(new Private()) > { > } > > I see no corresponding delete, so this is leaked. AbstractClient dtor? Done. > > Channel: Add includeSelfContact param to groupSelfContact and groupLocal/RemotePendingContacts. > > Commit message clearly refers wrongly to groupSelfContact(). Also, docs missing > for the includeSelfContact params. > > + if (!includeSelfContact) { > + return mPriv->groupRemotePendingContacts.values().toSet() - Contacts() > << groupSelfContact(); > + } > return mPriv->groupRemotePendingContacts.values().toSet(); > > Something like > > Contacts c = mPriv->whicheverContactsYoureReturning.values().toSet(); > if (!includeSelfContact) { > c.remove(groupSelfContact()); > } > return c; > > would be both cleaner (no duplicate mention of the source container) and faster > (no singleton set created and trivially iterated for the self contact). Done. > Otherwise looks good to me. I didn't look in Jeremy's big patches in detail, > just an overview; I trust you've already done that when inheriting the work. I did :)
+ * \param includeSelfContact Whether to include the self contact to the returned set. "in the returned set" I'd also explain it a bit more in the long form description of the methods. Because "self contact" is not really a common knowledge term. Something like * It is possible to omit the contact representing the local user, even if he/she * is in the set, by passing \c false as the parameter \a includeSelfContact. Otherwise good to go after merging the qt5 branch.
(In reply to comment #4) > + * \param includeSelfContact Whether to include the self contact to the > returned set. > > "in the returned set" > > I'd also explain it a bit more in the long form description of the methods. > Because "self contact" is not really a common knowledge term. > > Something like > > * It is possible to omit the contact representing the local user, even if > he/she * is in the set, by passing \c false as the parameter \a > includeSelfContact. > Done > Otherwise good to go after merging the qt5 branch. Thanks, will merge.
Changes merged upstream. It will be in tp-qt 0.9.0
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.