[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