Bug 38249 - Add API for the ChannelRequest.DelegateToPreferredHandler hint
Add API for the ChannelRequest.DelegateToPreferredHandler hint
Status: RESOLVED FIXED
Product: Telepathy
Classification: Unclassified
Component: tp-glib
unspecified
Other All
: medium enhancement
Assigned To: Telepathy bugs list
Telepathy bugs list
http://cgit.collabora.com/git/user/ca...
: patch
Depends on: 38240
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-13 06:43 UTC by Guillaume Desmottes
Modified: 2011-07-07 23:50 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
simple-channel-request: export tp_tests_simple_channel_request_dup_immutable_props() (1.92 KB, patch)
2011-06-24 05:21 UTC, Guillaume Desmottes
Details | Splinter Review
TpBaseClient: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249) (14.29 KB, patch)
2011-06-24 05:21 UTC, Guillaume Desmottes
Details | Splinter Review
add tp_account_channel_request_set_delegate_to_preferred_handler() (3.02 KB, patch)
2011-06-24 05:21 UTC, Guillaume Desmottes
Details | Splinter Review
TpAccountChannelRequest: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249) (14.01 KB, patch)
2011-07-05 02:39 UTC, Guillaume Desmottes
Details | Splinter Review
TpAccountChannelRequest: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249) (15.57 KB, patch)
2011-07-07 02:06 UTC, Guillaume Desmottes
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Guillaume Desmottes 2011-06-13 06:43:10 UTC
See bug #38240
Comment 2 Danielle Madeley 2011-06-13 07:45:58 UTC
Ignore that.
Comment 4 Guillaume Desmottes 2011-06-24 05:21:22 UTC
Created attachment 48376 [details] [review]
simple-channel-request: export tp_tests_simple_channel_request_dup_immutable_props()
Comment 5 Guillaume Desmottes 2011-06-24 05:21:26 UTC
Created attachment 48377 [details] [review]
TpBaseClient: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249)
Comment 6 Guillaume Desmottes 2011-06-24 05:21:30 UTC
Created attachment 48378 [details] [review]
add tp_account_channel_request_set_delegate_to_preferred_handler()
Comment 7 Danielle Madeley 2011-07-04 15:49:55 UTC
This is all good, but you forgot support for ::re-delegated in TpAccountChannelRequest for when someone has called *_and_handle_async().
Comment 8 Guillaume Desmottes 2011-07-05 02:39:38 UTC
Created attachment 48760 [details] [review]
TpAccountChannelRequest: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249)
Comment 9 Danielle Madeley 2011-07-06 16:23:45 UTC
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?
Comment 10 Guillaume Desmottes 2011-07-07 02:06:05 UTC
(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.
Comment 11 Guillaume Desmottes 2011-07-07 02:06:55 UTC
Created attachment 48843 [details] [review]
TpAccountChannelRequest: add org.freedesktop.Telepathy.ChannelRequest.DelegateToPreferredHandler support (#38249)
Comment 12 Danielle Madeley 2011-07-07 17:00:28 UTC
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.
Comment 13 Guillaume Desmottes 2011-07-07 23:50:13 UTC
Agreed that's better. I used that wording and merged the branch. Thanks for all the reviewing!