[Bug 29079] use TpBaseProtocol in Gabble

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Aug 2 14:33:53 CEST 2010


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

--- Comment #4 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-08-02 05:33:53 PDT ---
In the test:
> +    # (Only) 'account' is mandatory for IdentifyAccount()
> +    try:
> +        proto_iface.IdentifyAccount({})
> +    except dbus.DBusException, e:
> +        assertEquals(cs.INVALID_ARGUMENT, e.get_dbus_name())
> +    else:
> +        raise AssertionError("IdentifyAccount({}) should've returned error but didn't")

call_async(q, proto_iface, 'IdentifyAccount', {})
q.expect('dbus-error', method='IdentifyAccount', name=cs.INVALID_ARGUMENT)

> +  protocol = (TpBaseProtocol *) gabble_jabber_protocol_new ();

You could make g_j_p_n return a TpBaseProtocol* to avoid the cast?

In Protocol:
> +  static GOnce init = G_ONCE_INIT;
> +
> +  g_once (&init, _init_parameters, NULL);

I find g_once_init_enter() and g_once_init_leave() typically lead to more
obvious code. They're available since GLib 2.14.

In write-mgr-file:
> +    if (val && *val) \

if (tp_str_empty (val)), since telepathy-glib 0.11.1

(Previously, the coding style would have been: if (val != NULL && *val !=
'\0'))

I think it would be OK for write-mgr-file to assert if any of the immutable
properties aren't there. After all, we know that they're there :-)

> +      g_hash_table_foreach (ctx.fixed, write_rcc_property, &ctx);

Why not use GHashTableIter? It usually produces much more comprehensible code,
in my experience.

> +          gchar *kf_val = g_strdup_printf ("%llu", g_value_get_uint64 (val));

The format string should be: "%" G_GUINT64_FORMAT

Same for G_TYPE_INT64.

Also add a FIXME comment: when we depend on GLib 2.26, we can use
g_key_file_set_[u]int64, which I added in Gnome bug 614864.

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



More information about the telepathy-bugs mailing list