|
Description
Simon McVittie
2012-02-09 07:46:03 UTC
Created attachment 57940 [details] [review] Don't use tp_channel_call_when_ready, except in its regression tests Created attachment 57941 [details] [review] Use tp_proxy_prepare_async() to prepare connections in example code Created attachment 57942 [details] [review] Deprecate tp_channel_call_when_ready, tp_connection_call_when_ready Created attachment 57943 [details] [review] Deprecate tp_channel_is_ready and TpChannel:channel-ready Created attachment 57944 [details] [review] TpContact: stop using about-to-be-deprecated Connection API Created attachment 57945 [details] [review] Deprecate tp_connection_is_ready, TpConnection:connection-ready Created attachment 57946 [details] [review] inspect-cm example: stop using call_when_ready (Still to do: actually deprecate tp_connection_manager_call_when_ready()) All these patches so far look good. (In reply to comment #9) > All these patches so far look good. Thanks, merged. More to come... Created attachment 58024 [details] [review] tests/dbus/cm: use g_assert_no_error for better diagnostics --- I have a branch in progress to do this to the other tests, but this bit seemed likely to conflict. Created attachment 58025 [details] [review] Improve coverage of TpConnectionManager test * test prepare_async in every situation where we previously tested call_when_ready * test is_prepared (CORE) in the same situations * test get_invalidated in the same situations This brings us closer to being able to deprecate call_when_ready and is_ready. Created attachment 58026 [details] [review] tp_list_connection_managers: use tp_proxy_prepare_async We're about to deprecate call_when_ready, so let's not call it. Created attachment 58027 [details] [review] Deprecate tp_connection_manager_call_when_ready and is_ready Created attachment 58031 [details] [review] [next] Remove TpChannel-specific readiness, just use TpProxy preparation Created attachment 58032 [details] [review] [next] Remove TpConnection-specific readiness, just use TpProxy preparation gitwebs: http://cgit.freedesktop.org/~smcv/telepathy-glib/log?h=deprecate-when-ready-45842 http://cgit.freedesktop.org/~smcv/telepathy-glib/log?h=next-remove-cwr-45842 Still to do: remove tp_connection_manager_call_when_ready from next (after merging master, with the patches from this bug applied). Comment on attachment 58026 [details] [review] tp_list_connection_managers: use tp_proxy_prepare_async Review of attachment 58026 [details] [review]: ----------------------------------------------------------------- ::: telepathy-glib/connection-manager.c @@ +1676,5 @@ > size_t base_len; > gsize refcount; > gsize cms_to_ready; > unsigned getting_names:1; > + unsigned had_weak_object:1; Are we really still using this instead of gboolean in new code? @@ +1732,4 @@ > g_ptr_array_add (list_context->arr, NULL); > cms = (TpConnectionManager **) list_context->arr->pdata; > > + if (!list_context->had_weak_object || list_context->weak_object != NULL) I don't understand this. Call the callback if: * there was no weak object (regardless of whether it's been disposed already or not) * OR, there is a weak object around right now So the only case where the callback won't get called is if: * had_weak_object is TRUE * AND, the weak object is not around. Okay so this now makes sense: it'll only call the callback if the weak object is still around. Would it be possible to get a comment to explain this a bit more though please? Every other patch looks good to me. If you could add another comment I'd appreciate it. (In reply to comment #18) > > unsigned getting_names:1; > > + unsigned had_weak_object:1; > > Are we really still using this instead of gboolean in new code? I was going for "if there's already a bitfield extend it, if not, add a gboolean". > Okay so this now makes sense: it'll only call the callback if the weak object > is still around. Would it be possible to get a comment to explain this a bit > more though please? /* If we never had a weak object anyway, call the callback. * If we had a weak object when we started, only call the callback * if it hasn't died yet. */ I regret introducing the weak_object parameter, and look forward to the glorious GIO future, with async results, cancellables where we really really need them, and "shut up and use TpWeakRef as your user_data" for the few cases where the weak_object semantics are actually useful. Deprecation in 0.17.6, removal in next. Leaving the bug open to remove the CM readiness in next. Created attachment 58053 [details] [review] [next] Remove tp_connection_manager_call_when_ready, is_ready (In reply to comment #20) > I was going for "if there's already a bitfield extend it, if not, add a > gboolean". I think we want to replace all these with gbooleans in next. > /* If we never had a weak object anyway, call the callback. > * If we had a weak object when we started, only call the callback > * if it hasn't died yet. */ Okay, thanks. Comment on attachment 58053 [details] [review] [next] Remove tp_connection_manager_call_when_ready, is_ready Review of attachment 58053 [details] [review]: ----------------------------------------------------------------- Perfecto! (In reply to comment #23) > (In reply to comment #20) > > I was going for "if there's already a bitfield extend it, if not, add a > > gboolean". > > I think we want to replace all these with gbooleans in next. Bug #48544. (In reply to comment #24) > Perfecto! Fixed in next. |
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.