Bug 38249 - Add API for the ChannelRequest.DelegateToPreferredHandler hint
Summary: Add API for the ChannelRequest.DelegateToPreferredHandler hint
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/ca...
Whiteboard:
Keywords: 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!


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.