Bug 33271

I have an start of implementation of this in http://git.collabora.co.uk/?p=user/cassidy/telepathy-mission-control;a=shortlog;h=refs/heads/re-handle-33271

I re-implemented all the logic rather than reusing the existing dispatching code as redispatching is much simpler than the general dispatching which is already pretty complex.

Looks reasonable to me.
Make it so.

Merged to master. I keep this bug opened while we don't implement the stable API.

I updated to latest spec and implemented CD.ReEnsureChannel().

http://git.collabora.co.uk/?p=user/cassidy/telepathy-mission-control;a=shortlog;h=refs/heads/re-dispatch-33271

Here is the cleaned branch implementing the stable API:
http://cgit.collabora.co.uk/git/user/cassidy/telepathy-mission-control/log/?id=refs/heads/re-dispatch-33271

We need a tp-glib 0.15.1 release (and a dep bump) before merging it but it can already be reviewed.

+ /* All the channels should belong to the same account, so we just need
+ * to get the connection for the first one. */

o rly? The spec says nothing about this?

+ g_object_get (self->priv->master, "account-manager", &am, NULL);
+ g_assert (am != NULL);
+
+ account = mcd_account_manager_lookup_account_by_path (am, chan_account);
+ g_assert (account != NULL);

I'm not sure about asserting on failures. With non-fatal assertions MC will continue to run but PresentChannels won't return? I realise it's only going to fail in a very odd case, but still.

Otherwise looks fine.

(In reply to comment #6)
> + /* All the channels should belong to the same account, so we just need
> + * to get the connection for the first one. */
> 
> o rly? The spec says nothing about this?

I think that make sense, don't you? I'll update the spec if you agree that's a
sane requierement.

> + g_object_get (self->priv->master, "account-manager", &am, NULL);
> + g_assert (am != NULL);

There are a loads of exactly the same assertion in the code.

> + account = mcd_account_manager_lookup_account_by_path (am, chan_account);
> + g_assert (account != NULL);
> 
> I'm not sure about asserting on failures. With non-fatal assertions MC will
> continue to run but PresentChannels won't return? I realise it's only going to
> fail in a very odd case, but still.

I changed to g_return_if_fail() for the other assertions.

http://cgit.collabora.co.uk/git/user/cassidy/telepathy-mission-control/log/?id=refs/heads/re-dispatch-33271
now implements new new version of the spec (bug #37109)

(In reply to comment #8)
> http://cgit.collabora.co.uk/git/user/cassidy/telepathy-mission-control/log/?id=refs/heads/re-dispatch-33271
> now implements new new version of the spec (bug #37109)

Did you mean to include the doc/reference/*/tmpl/*.sgml files?

+static ChannelToDelegate *
+channel_to_delegate_new (DelegateChannelsCtx *ctx,
+  McdAccount *account,
+  McdChannel *channel)

then

+static void
+free_not_delegated_error (gpointer data)
+{
+    g_boxed_free (TP_STRUCT_TYPE_NOT_DELEGATED_ERROR, data);
+}

What's up with the indentation?

Okay, everything else looks fine. In an ideal world I'd like another test or two for multiple channels which one fails and one succeeds in delegation and stuff like that, but if you cba... :-)

Merge away, once the spec branch is merged.

Oh, also, could you give me spec branch which adds a comment to DelegateChannels and PresentChannels saying which version of MC this was released in?

(In reply to comment #9)
> (In reply to comment #8)
> > http://cgit.collabora.co.uk/git/user/cassidy/telepathy-mission-control/log/?id=refs/heads/re-dispatch-33271
> > now implements new new version of the spec (bug #37109)
> 
> Did you mean to include the doc/reference/*/tmpl/*.sgml files?

Rah stupid files; removed.

> +static ChannelToDelegate *
> +channel_to_delegate_new (DelegateChannelsCtx *ctx,
> +  McdAccount *account,
> +  McdChannel *channel)
> 
> then
> 
> +static void
> +free_not_delegated_error (gpointer data)
> +{
> +    g_boxed_free (TP_STRUCT_TYPE_NOT_DELEGATED_ERROR, data);
> +}
> 
> What's up with the indentation?

Yeah some part of the file had it wrong; I fixed it.

> Merge away, once the spec branch is merged.

We need a tp-glib release first.

> Oh, also, could you give me spec branch which adds a comment to
> DelegateChannels and PresentChannels saying which version of MC this was
> released in?

Ok, I'll do one once it's merged.

Merged to master; will be in MC 5.7.12

Due to the addition of a telepathy-glib 0.15 dependency, this isn't suitable for 5.7.x/5.8.x; it can either be changed to use an internal copy of that interface, or be added in 5.9.0. On the assumption that the latter is what we want, I've branched 5.8.x from just before Guillaume's branch was merged.