[Telepathy] Some more tp-glib feedback

[Simon McVittie]

On Thu, 29 Mar 2007 at 19:21:22 +0100, Simon McVittie wrote:
> On Thu, 29 Mar 2007 at 19:27:20 +0200, Sjoerd Simons wrote:
> > * It's not documented that get_unique_connection_name may be NULL
> 
> Will fix.

Fixed in Darcs.

> > * tp_handle_ref has changed to be void.. Can we assume it shouts when you try
> >   to ref an invalid handle ? I used the return value to assert a handle was 
> >   valid while in the channel constructors.
> 
> Yes, it is expected to assert (well, g_return_if_fail).

Documented in Darcs, sort of (in the form "it is an error if...").

> > * Prop protocol of base-connection is used internally in contrast to what's
> >   stated in the doc..  It's even mandatory !!
> 
> "Unused internally" was misleading; I believe it was meant to mean "we
> only use it to reply to GetProtocol". It's become untrue in any case, we
> use it for the object path and bus name too.

Fixed in Darcs.

> > * start_connecting always sets connecting after start_connecting.. Maybe we
> >   should set it to connecting just before calling start_connecting? So we can 
> >   handle CM's that don't (have) to connect async.
> >
> >   But that way if start connecting failed we would have -> connecting ->
> >   disconnected signals during the Connect() call?
> 
> Or, perhaps the base class should set CONNECTING afterwards, or
> DISCONNECTED on failure, but only if it's changing the state from NEW? That
> way start_connecting could set CONNECTED (or DISCONNECTED) if it wanted to.

Implemented in Darcs, and documented.

> Hmm... at the moment we guarantee that the state will pass through
> CONNECTING, so we always call channel factory callbacks etc. for it. We
> should probably preserve that invariant too.

Also implemented in Darcs: if you try to go straight from NEW to
CONNECTED, the state will momentarily change to CONNECTING first (just
to send the signal and trigger the callbacks).

> Setting CONNECTING before start_connecting runs (after verifying that
> at least Gabble and tp-sofiasip will behave correctly with that re-ordering)
> seems the sanest thing to do.

... but doing that would in fact have broken Gabble completely, so I
didn't.

> > * Calling TP_SVC_CHANNEL_INTERFACE_GROUP_CLASS on a Objectclass implementing 
> >   the interface asserts an invalid cast. Shouldn't that work or am i doing 
> >   something silly here :) 
> 
> I think that ought to work. I'll check.

Doesn't work, you're quite right... and it never has and never will (the
underlying function doesn't support interfaces)! I'm going to
remove the corresponding macros from the telepathy-glib interfaces
(specifically, TP_..._CLASS and TP_IS_..._CLASS) - they don't support
interfaces, only real classes.