[Bug 24653] Should implement ContactCapabilities

[bugzilla-daemon at freedesktop.org]

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

Will Thompson <will.thompson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |review+ with a couple of
                   |                            |suggestions.

--- Comment #3 from Will Thompson <will.thompson at collabora.co.uk> 2011-03-17 09:52:15 PDT ---
This looks fine. I've got a couple of nitpicks, but neither is a blocker.

+      const gchar *client_name = g_value_get_string (va->values + 0);
+      const GPtrArray *filters = g_value_get_boxed (va->values + 1);
+      const gchar * const * cap_tokens = g_value_get_boxed (va->values + 2);

I could suggest using tp_value_array_unpack here? I don't know how much clearer
it would actually be.

  if (!announce_self_caps (self, &error))
    {
      gabble_capability_set_free (before);
      dbus_g_method_return_error (context, error);
      g_error_free (error);
      return;
    }

  if (!gabble_capability_set_equals (before, after))
    {
      _emit_contact_capabilities_changed (self, base->self_handle);
    }

I'm tempted to say that the former block should be inside the latter — why
would we want to publish updated caps if they haven't changed? I know this
isn't a regression in your 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.
You are the assignee for the bug.