[Bug 34796] Implement ContactInfo

[bugzilla-daemon at freedesktop.org]

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

--- Comment #3 from Will Thompson <will.thompson at collabora.co.uk> 2011-02-28 04:11:34 PST ---
Review of attachment 43906:
 --> (https://bugs.freedesktop.org/review?bug=34796&attachment=43906)

Great! One functional issue, and a few nitpicks:

::: src/idle-connection.c
@@ +77,3 @@
 G_DEFINE_TYPE_WITH_CODE(IdleConnection, idle_connection,
TP_TYPE_BASE_CONNECTION,
         G_IMPLEMENT_INTERFACE(TP_TYPE_SVC_CONNECTION_INTERFACE_ALIASING,
_aliasing_iface_init);
+        G_IMPLEMENT_INTERFACE(TP_TYPE_SVC_CONNECTION_INTERFACE_CONTACT_INFO,
idle_contact_info_iface_init);

I think you also want to add TP_IFACE_CONNECTION_INTERFACE_CONTACT_INFO to
interfaces_always_present later in the file. This will make it appear in the
Connection's Interfaces property
<http://telepathy.freedesktop.org/spec/Connection.html#Property:Interfaces>
which the client-side bindings use to determine whether or not an interface is
implemented. Without this, clients may not realise that they can use these
methods.

::: src/idle-contact-info.c
@@ +48,3 @@
+ */
+static void _insert_contact_field(GPtrArray *contact_info, const gchar
*field_name, const gchar * const *field_params, const gchar * const
*field_values) {
+    gchar *field_name_down = g_ascii_strdown (field_name, -1);

Nitpick: given that we only ever pass strings provided by Idle itself as
'field_name', can't we just assume they're lower-case (or else the code calling
_insert_contact_field is buggy)?

@@ +108,3 @@
+
+   
tp_svc_connection_interface_contact_info_return_from_refresh_contact_info(context);
+}

You could just omit the stub implementations of GetContactInfo,
RefreshContactInfo and SetContactInfo entirely: tp-glib would fall back to
returning a NotImplemented error.

@@ +178,3 @@
+        g_error_free(error);
+        return;
+    }

If tp_handle_is_valid said that 'contact' was valid, tp_handle_inspect() on it
will always return non-NULL.

::: src/idle-contact-info.h
@@ +35,3 @@
+extern TpDBusPropertiesMixinPropImpl *idle_contact_info_properties;
+void idle_contact_info_properties_getter (GObject *object, GQuark interface,
+    GQuark name, GValue *value, gpointer getter_data);

Looks like you didn't actually implement the two properties:
<http://telepathy.freedesktop.org/spec/Connection_Interface_Contact_Info.html#properties>.
But since their values would be '0' and '[]' anyway, this doesn't seem like a
big deal. :)

-- 
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.