[Bug 34796] Implement ContactInfo

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Debarshi Ray <debarshi.ray at gmail.com> 2011-02-28 06:17:49 PST ---
(In reply to comment #3)
> Review of attachment 43906 [details]:
> 
> 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.

Done.

> ::: 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)?

You are right. I had thought about it too. Thing is that I copied the function
and the comments above it verbatim from telepathy-gabble. Apart from some style
fixes I did not change the code to avoid fragmented copies of the same code.
But I have no strong opinion about this. :-)

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

I intend to implement those methods. :-)

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

Done.

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

Ok. I will look into this.

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