[Bug 28199] port Gabble to TpBaseContactList

[bugzilla-daemon at freedesktop.org]

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

--- Comment #7 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-08-18 05:43:08 PDT ---
I've done a first review (patch by patch) of this. Keep in mind that I'm not a
Gabble roster expert (I never really hacked on it) so I would have probably
missed subtil issues in its logic.

_gabble_roster_item_free() should free item->publish_request

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=commitdiff;h=a07e273d4694040ab4ed287c97bb8a99fcf46b90
I like the idea. Would be good to have somthing similar in glib. Maybe using
another result object implementing the GAsyncResult interface so we wouldn't
have to abuse the res pointer.

+  /* list of GSimpleAsyncResult */
+  GSList *results;
I'd say if the result objects are owned or borrowed

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=commitdiff;h=6e66629b4958990d197c15e03020f7c958f2fe25
why not keep both variants and so test the old API as well as you did with
other tests?
Same for other tests

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=commitdiff;h=a132acd51f6bf7d9c1cec5c801704f5305fdcf03
if the change didn't stick, shouldn't conn.Contacts.GetContactAttributes()
return the "people starting with A" group as well?

I try to use "pyflakes" when adding new tests. For example pyflakes
tests/twisted/roster/edit-before-roster.py returns a bunch of trivial warnings
that could easily be fixed.
Same for tests/twisted/roster/authorize.py

+  if (priv->pre_authorized != NULL)
+    {
+      tp_handle_set_destroy (priv->pre_authorized);
+      priv->pre_authorized = NULL;
+    }

Why not use your shiny tp_clear_pointer() ? :)

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