[Bug 27175] Make TpMessage usable in clients

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Thu Oct 28 16:57:24 CEST 2010


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

--- Comment #11 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-10-28 07:57:24 PDT ---
Drive-by comments rather than a thorough review:

Yes this seems to be on the right track.

In message.h:

> +#include <telepathy-glib/base-connection.h>

I'd prefer it if this (and the method that needs it) only appeared in
cm-message.h (which would be the one included by message-mixin.h).

> +  /* Now that @self has stolen @message's parts, replace them with a stub to
> +   * keep tp_message_destroy happy.

This is, er, suspicious, now that messages are refcounted... could we use
TpVariantMap's cheap copy-on-write mechanism (Bug #30423) instead?

(That'd mean the internals of a TpMessage being GPtrArray<TpVariantMap>, and
mapping to and from dbus-glib whenever we touch D-Bus, in the medium term. I
think that'd be worthwhile, but I can see that it's not on the shortest path
for this bug.)

> +# Header files to ignore when scasignalled

o rly? :-)

(In reply to comment #8)
> I didn't make the initial_parts / size_hint distinction in
> tp_message_client_new() as I'm not sure what it really buys us.

That's fair enough; size-hint was just a premature optimization anyway.

I'm not sure whether initial-parts really makes sense as a construct-time
property either; callers (in e.g. language bindings) can always just call
tp_message_append_part()?

More specifically, tp_message_new (initial_parts=n) is exactly equivalent to
creating a new TpMessage with only the required headers part, then calling
tp_message_append_part() n-1 times; so I'd be inclined to say that it's OK to
have a _new parameter that isn't a construct-time property, just as syntactic
sugar similar to tp_client_message_text_new().

tp_client_message_text_new() should be renamed to tp_client_message_new_text(),
because it's conceptually a secondary constructor "ClientMessage.new_text()" in
the GObject object model, and GObject constructors start with "new".

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



More information about the telepathy-bugs mailing list