http://cgit.collabora.com/git/user/danni/telepathy-spec.git/log/?h=redelegate-hint-38240

Ignore that.

http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=delegate-preferred-38249

Created attachment 48376 [details] [review]
simple-channel-request: export tp_tests_simple_channel_request_dup_immutable_props()

Created attachment 48377 [details] [review]
TpBaseClient: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249)

Created attachment 48378 [details] [review]
add tp_account_channel_request_set_delegate_to_preferred_handler()

This is all good, but you forgot support for ::re-delegated in TpAccountChannelRequest for when someone has called *_and_handle_async().

Created attachment 48760 [details] [review]
TpAccountChannelRequest: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249)

Review of attachment 48760 [details] [review]:

Did you forget to add account-channel-request-internal ?

::: docs/reference/Makefile.am
@@ +65,3 @@
 IGNORE_HFILES=\
     add-dispatch-operation-context-internal.h \
+    account-channel-request--internal.h \

Double dash?

::: telepathy-glib/account-channel-request.c
@@ +851,3 @@
+  g_assert (TP_IS_CHANNEL (channel));
+  g_assert_cmpstr (tp_proxy_get_object_path (channel), ==,
+

Are assertions appropriate here, or would g_return_if_fail() be better?

@@ +1673,3 @@
+ * channels.
+ *
+ *

Is there a word missing here?

@@ +1682,3 @@
+ * channel.
+ *
+ * When receiving a request containing this hint, @self will automatically

Consider a See Also: to the TpBaseClient description?

(In reply to comment #9)
> Review of attachment 48760 [details] [review]:
> 
> Did you forget to add account-channel-request-internal ?

I did :(

> ::: docs/reference/Makefile.am
> @@ +65,3 @@
>  IGNORE_HFILES=\
>      add-dispatch-operation-context-internal.h \
> +    account-channel-request--internal.h \
> 
> Double dash?

Ooops, fixed;

> ::: telepathy-glib/account-channel-request.c
> @@ +851,3 @@
> +  g_assert (TP_IS_CHANNEL (channel));
> +  g_assert_cmpstr (tp_proxy_get_object_path (channel), ==,
> +
> 
> Are assertions appropriate here, or would g_return_if_fail() be better?

g_return_if_fail() is probably better indeed; changed.

> @@ +1673,3 @@
> + * channels.
> + *
> + *
> 
> Is there a word missing here?

??

Btw, I renamed it to tp_account_channel_request_set_delegated_channel_callback
(singular) as there is only one channel being delegated in this case.

> @@ +1682,3 @@
> + * channel.
> + *
> + * When receiving a request containing this hint, @self will automatically
> 
> Consider a See Also: to the TpBaseClient description?

done.

Created attachment 48843 [details] [review]
TpAccountChannelRequest: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249)

Review of attachment 48843 [details] [review]:

::: telepathy-glib/account-channel-request.c
@@ +1676,3 @@
+ * tp_account_channel_request_create_and_handle_channel_async () or
+ * tp_account_channel_request_ensure_and_observe_channel_async() has been
+ * the org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler

This is really awkwardly worded, it took me 4 rereadings to work out what you were saying.

Perhaps simply, "@callback may be called any time after (and only after) requesting and handling the channel (i.e. you have called create_and_handle or ensure_and_handle).

N.B. you have written ensure_and_observe, which is wrong.

Agreed that's better. I used that wording and merged the branch. Thanks for all the reviewing!