[Bug 27872] TpBaseClient Handler support

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon May 24 13:31:35 CEST 2010


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

--- Comment #8 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-05-24 04:31:35 PDT ---
+++ b/tests/dbus/base-client.c
> +#include <telepathy-glib/handle-channels-context-internal.h>

Why? If possible I'd prefer it if regression tests didn't use internal API.

> +  param_spec = g_param_spec_uint64 ("user-action-time",

This (and friends) should probably be an int64 that happens to be positive,
even though we get a uint64 from D-Bus (which was a mistake in tp-spec).

It'd be good to document it in terms of the stuff you recently merged to
tp-spec, too (with the 0 and G_MAXINT64 special cases).

Non-blocking trivia
===================

> +      g_warning ("Disposing a context in the %s state",

Could be WARNING().

> +  void (*chain_up) (GObject *) =
> +    ((GObjectClass *) \
> +      tp_handle_channels_context_parent_class)->constructed;

Unnecessary backslash.

> +  GList *l;

In future I'd prefer it if you'd avoid lower-case L as a variable name - it
looks a lot like 1.

> +#include "telepathy-glib/handle-channels-context-internal.h"
> +#include "telepathy-glib/handle-channels-context.h"

Please put the public header first; the point of doing that is to verify that
the public header is self-contained.

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