GNOME Bugzilla – Bug 648822
Port Empathy to Folks 0.5.1
Last modified: 2011-06-06 16:53:43 UTC
Folks 0.5.1 (not yet released) will break API. This bug will serve as a soft blocker for that release, since Empathy is one of our primary dependents.
This should be done for GNOME 3.2.
Created attachment 187397 [details] [review] Port Empathy to Folks 0.5.1. This is the patches from git branch: http://cgit.collabora.co.uk/git/user/treitter/empathy.git/log/?h=bgo648822-rebase6
Review of attachment 187397 [details] [review]: Looks good overall. A few comments below, mostly for what looks like copy-paste errors. Splinter's misbehaving a little, so here are some extra comments it doesn't seem to want to let me make: empathy-individual-widget.c:1109 (individual_is_user): Shouldn't retval be returned here? empathy-individual-widget.c:1726 (personas_changed_cb): Might be an idea to add a comment that iter is being re-used. empathy-individual-store.c:480 (individual_store_add_individual): s/char/gchar/ empathy-individual-widget.c:1821 (personas_changed_cb): removed is guaranteed to be non-NULL, so the check that it's non-NULL is unnecessary ::: libempathy-gtk/empathy-individual-dialogs.c @@ +245,3 @@ + +while_finish: + tp_clear_object (&iter); Shouldn't that be persona instead of iter? ::: libempathy-gtk/empathy-individual-menu.c @@ +86,2 @@ personas = folks_individual_get_personas (individual); + iter = gee_iterable_iterator (GEE_ITERABLE (personas)); Might be useful to comment that you're re-using the same iter in two places in the function. ::: libempathy/empathy-contact.c @@ +884,3 @@ personas = folks_individual_get_personas (individual); + iter = gee_iterable_iterator (GEE_ITERABLE (personas)); + while (gee_iterator_next (iter)) Shouldn't this be tp_clear_object()? @@ +903,3 @@ + + if (persona_found) + goto finished; If this goto is taken, I don't think iter ever gets cleared. @@ +906,3 @@ } } + tp_clear_object (&iter); Could use tp_clear_object() here. ::: libempathy/empathy-individual-manager.c @@ +523,3 @@ + if (empathy_contact_manager_get_flags_for_connection (manager, conn) & + EMPATHY_CONTACT_LIST_CAN_BLOCK) + success = TRUE; Could you not just set retval directly here? @@ +574,3 @@ + g_object_unref (contact); + } + tp_clear_object (&iter); I think this should be persona rather than iter. ::: libempathy/empathy-utils.c @@ +789,3 @@ + TpContact *contact = NULL; + + if (empathy_folks_persona_is_interesting (persona)) If this branch isn't taken, the persona_store will leak a reference.
Created attachment 187599 [details] [review] Port Empathy to Folks 0.5.1 (try 2) Patches from branch: http://cgit.collabora.co.uk/git/user/treitter/empathy.git/log/?h=bgo648822-rebase8
(In reply to comment #3) > Review of attachment 187397 [details] [review]: > > Looks good overall. A few comments below, mostly for what looks like copy-paste > errors. I've fixed all the other issues. A couple comments: > empathy-individual-widget.c:1821 (personas_changed_cb): removed is guaranteed to be non-NULL, so the check that it's non-NULL is unnecessary That's true for Folks itself, but personas_changed_cb() gets called manually with a NULL removed from individual_update(), so removing the check turns this into a crasher. @@ +906,3 @@ } } + tp_clear_object (&iter); Could use tp_clear_object() here. Do you mean g_clear_object()? I've changed all new additions of tp_clear_object to g_clear_object in the new branch, since we already depend upon the required version of GLib. Other than that, all changes commits appear as trivial fixup commits on the new branch.
(In reply to comment #5) > > empathy-individual-widget.c:1821 (personas_changed_cb): removed is guaranteed > to be non-NULL, so the check that it's non-NULL is unnecessary > > That's true for Folks itself, but personas_changed_cb() gets called manually > with a NULL removed from individual_update(), so removing the check turns this > into a crasher. Fair enough. > @@ +906,3 @@ > } > } > + tp_clear_object (&iter); > > Could use tp_clear_object() here. > > Do you mean g_clear_object()? I've changed all new additions of tp_clear_object > to g_clear_object in the new branch, since we already depend upon the required > version of GLib. This is the first I've heard of g_clear_object(). Good to see that it's finally in GLib! I guess I do mean g_clear_object() then. :-) > Other than that, all changes commits appear as trivial fixup commits on the new > branch. All the fixups look good to me, so as far as I'm concerned the branch can be committed. I don't know if Guillaume wants to take a look first though.
Folks 0.5.1 has been released. Any reason to not apply this, Guillaume?
Update jhbuild and go for it. :)
I tried to build 0.5.1 but failed, see bug #650552
Created attachment 189032 [details] [review] Port Empathy to Folks 0.5.1 (try 3) Refresh with the latest from git master (to coincide with Folks 0.5.2): http://cgit.collabora.com/git/user/treitter/empathy.git/log/?h=bgo648822-rebase10 The changes vs. the last branch is that this updates an old-API usage of folks_individual_get_personas() and folks_individual_new() added to master since the last rebase.
Review of attachment 189032 [details] [review]: ++
(In reply to comment #11) > Review of attachment 189032 [details] [review]: > > ++ Merged. I've also updated jhbuild to Folks 0.5.2. commit 24bfa52b2a137a2bdafceba1fee199a06a9509ec Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Thu May 12 14:37:03 2011 -0700 Require Folks 0.5.1 for the API updates. Closes: bgo#648822 - Port Empathy to Folks 0.5.1 configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) commit 018ba00ead80bed3238273a772ec1fd1c55cf58e Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Tue May 10 16:41:04 2011 -0700 Only retrieve server-stored groups for Individuals with TpContacts. Helps: bgo#648822 - Port Empathy to Folks 0.5.1 libempathy-gtk/empathy-individual-store.c | 25 +++++++++++++------------ 1 files changed, 13 insertions(+), 12 deletions(-) commit a6f6a3b008a9da3e437488b480fdbb2b5d07e9fd Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Thu May 5 11:05:04 2011 -0700 Adapt to API change in FolksIndividual::personas-changed. Helps: bgo#648822 - Port Empathy to Folks 0.5.1 libempathy-gtk/empathy-individual-widget.c | 41 ++++++++++++++++++++++----- libempathy-gtk/empathy-persona-store.c | 26 +++++++++++++----- 2 files changed, 52 insertions(+), 15 deletions(-) commit a922c5d3011ce0be0d57f26835d20758cf4c320e Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Wed May 4 14:22:36 2011 -0700 Adapt to API change in FolksIndividualAggregator::individuals-changed. Helps: bgo#648822 - Port Empathy to Folks 0.5.1 libempathy/empathy-individual-manager.c | 31 ++++++++++++++++++++++--------- 1 files changed, 22 insertions(+), 9 deletions(-) commit 0e80ea37d08505e886ef61b03fbb606a8ea3ac36 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Mon May 2 13:17:23 2011 -0700 Adapt to API change in folks_group_details_get_groups(). Helps: bgo#648822 - Port Empathy to Folks 0.5.1 libempathy-gtk/empathy-groups-widget.c | 7 ++++--- libempathy-gtk/empathy-individual-store.c | 21 ++++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) commit a8833fd63b3bc3856cbd885158cba6b8fca1225a Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Mon May 2 12:58:04 2011 -0700 Change the type of EmpathyContact.priv.groups to GeeHashSet. This is to adjust to the newer API for folks_group_details_set_groups(). It's also slightly cleaner than using a hash table to implement a set. Helps: bgo#648822 - Port Empathy to Folks 0.5.1 libempathy/empathy-contact.c | 16 +++++++--------- 1 files changed, 7 insertions(+), 9 deletions(-) commit d4989c9be7b3c6fa016d703e2a32774d5f74f836 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Fri Apr 29 13:57:05 2011 -0700 Adapt to API change in folks_backend_get_persona_stores(). Helps: bgo#648822 - Port Empathy to Folks 0.5.1 libempathy/empathy-individual-manager.c | 6 ++++-- libempathy/empathy-utils.c | 15 +++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) commit 202af99a8fe154b9d03c2d569287fa01f70c0bc6 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Wed Jun 1 10:54:40 2011 -0700 Adapt to API change in FolksIndividual constructor. Helps: bgo#648822 - Port Empathy to Folks 0.5.1 src/empathy-invite-participant-dialog.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) commit 736b4f3d04f1e826dd8252fed88a7445b52ad461 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Fri Apr 29 13:33:04 2011 -0700 Adapt to API break in folks_individual_get_personas. Helps: bgo#648822 - Port Empathy to Folks 0.5.1 libempathy-gtk/empathy-individual-dialogs.c | 15 +- .../empathy-individual-information-dialog.c | 13 +- libempathy-gtk/empathy-individual-linker.c | 70 +++++--- libempathy-gtk/empathy-individual-linker.h | 2 +- libempathy-gtk/empathy-individual-menu.c | 93 ++++++---- libempathy-gtk/empathy-individual-store.c | 121 ++++++++----- libempathy-gtk/empathy-individual-view.c | 70 +++++--- libempathy-gtk/empathy-individual-widget.c | 193 +++++++++++++------- libempathy-gtk/empathy-linking-dialog.c | 14 +- libempathy-gtk/empathy-persona-store.c | 27 ++- libempathy-gtk/empathy-ui-utils.c | 57 ++++--- libempathy/empathy-contact.c | 44 +++-- libempathy/empathy-individual-manager.c | 82 +++++---- libempathy/empathy-individual-manager.h | 2 +- libempathy/empathy-utils.c | 61 +++++-- libempathy/empathy-utils.h | 2 +- src/empathy-invite-participant-dialog.c | 43 +++-- 17 files changed, 575 insertions(+), 334 deletions(-) commit faa40483fd000099a0593c09d0e92b938beaaaa7 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Mon May 2 17:21:04 2011 -0700 Don't conflate TpfPersona and FolksPersona. libempathy-gtk/empathy-individual-store.c | 4 ++-- libempathy-gtk/empathy-individual-widget.c | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) commit a17f14769b6867e240f1f79288ed1d33ec51760b Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Mon May 2 17:05:19 2011 -0700 Don't shadow the global definition of 'log'. libempathy-gtk/empathy-chat.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)