[Bug 43223] Resolve API/ABI break TODO issues before 0.9.0 (qt4/qt5) release

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=43223

Olli Salli <ollisal at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ollisal at gmail.com
         Depends on|                            |43222

--- Comment #2 from Olli Salli <ollisal at gmail.com> 2011-11-25 04:04:12 PST ---
(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.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.