[Telepathy] Re: Some tp-glib feedback
simon.mcvittie at collabora.co.uk
Thu Mar 29 04:47:30 PDT 2007
(Redirected to telepathy at l.f.o from private mail)
On Thu, 29 Mar 2007 at 12:49:24 +0200, Sjoerd Simons wrote:
> * TpDynamicHandleRepo
> - Not documented that default-normalize-context and normalize-function
> properties are not mandatory or what their default is
Good point, I'll fix that. For reference, the default normalize function
is (equivalent to) g_strdup and the default default-normalize-context is
> * TpBaseConnection
> - TpBaseConnectionProcWithError start_connecting says that it must calculate
> and ref the self_handle. But salut can only calculate it during the
> connection (as soon as the name is estabished on dns-sd)
I think it should be OK to change start_connecting's behaviour to be
"by the time you call t_b_c_change_status (CONNECTED), you must have
calculated and reffed the self handle".
> - For some reason the instance fields are missing from the documentation
I'm puzzled by that too, but will try to fix it.
> - Reading the documentation in the header it's not clear which instance
> fields are meant to be set by the base class and which by the subclass
Fair point, I'll fix that.
> * None of the *_return_from_<whatever> functions end up in the documentation.
I'll see what I can do. More code-generation-fu...
I need to document the generated code in a general way, really - a
chapter in the API docs entitled "How to use the TpSvc... GInterfaces"
> * ERROR_IF_NOT_CONNECTED_ASYNC would be nice to have as a utility on the base
You'll probably still want a #define, the properly namespaced name will be a
bit unwieldy... but yes.
> * tp_channel_factory_iface_request takes a guint handle, instead of TpHandle
> * tp_channel_factory_iface_request, the request argument must be include if the
> signalled new-channel's handle is 0.. but handles can't be 0 right? Also
> handle and handle_type can be 0, but i'm unsure how to handle those cases (or
> why they would ever be 0)
The channel might not really have a handle, which is modelled as "handle
0 of type 0". This asks for a new "anonymous" channel: for instance, media
channels are created with handle/type 0, then you add the callee to the
channel afterwards. If Salut chat rooms are unnamed but just exist as a
tuple of (multicast IP, port, key) or something, then they should have
"handle" 0 too.
> * The Test mixin functions want a gobject instance, the group mixin
> functions want a TpSvcChannelInterfaceGroup.. Would be good to be more
> consistent here :)
Perhaps they should both take a gpointer to avoid unnecessary casts...
The motivation for some of the group mixin functions taking a
TpSvcChannelInterfaceGroup* was to make them usable as implementations
for the GInterface methods straight away - some of the group methods
were also public API though, so I changed the rest of the group API in
order to be consistent. With hindsight, that was probably a bad move.
Simon McVittie, Collabora Ltd.: http://www.collabora.co.uk/
More information about the Telepathy