[Bug 38142] proxy factories

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=38142

--- Comment #71 from Will Thompson <will.thompson at collabora.co.uk> 2011-08-04 09:18:59 PDT ---
• Deprecate tp_account_manager_ensure_account()

Speaking of which: why is tp_account_manager_ensure_account() not just a
wrapper around the equivalent call to
tp_simple_client_factory_ensure_account()? I guess it's down to the fact that
the former doesn't return a ref, whereas the latter does? Annoying if so—but
does tp_account_manager_ensure_account() need to fire off a call to
tp_proxy_prepare_async()?

(Oh! for an autorelease pool.)

+  /* Lazy init self->priv->account */

English nitpick: “lazily init[ialize]”.

• TpBaseClient: prepare TpAccount, TpConnection and TpChannels with factory
desired features

This patch is ~1300 lines of changes and ends with

+#if 0
+
+  FIXME: Disabled for now because it is racy. If channel path is already
removed
+  from the bus when AddDispatchOperation implementation starts, it will return
a
+  dbus error.
+

I'm sorry, I can't bring myself to trust a patch which “refactor[s] completely
the way TpBaseClient prepares its proxies” whose only addition to the test
suite is to comment out part of a test case. ;) I haven't reviewed this patch,
or “TpBaseClient: add constructors with factory instead of AM”

• Deprecate tp_base_client_add_*_features()

  * @account: a #TpAccount with %TP_ACCOUNT_FEATURE_CORE, and any other
- *  features added via tp_base_client_add_account_features(), prepared if
+ *  features added via tp_base_client_add_account_features(), or
+ *  tp_simple_client_factory_add_account_features(), prepared if
  *  possible

The comma after tp_base_client_add_account_features() breaks the sentence.
Ditto all the other places this wording is used.

While we're deprecating things, can you use G_GNUC_DEPRECATED_FOR()—presumably
via another _TP_GNUC_DEPRECATED_FOR() wrapper—instead?

• Add tp_account_manager_set_default()

+ * Note that @manager must use the default dbus-daemon as returned by
+ * tp_dbus_daemon_dup()

#TpDBusDaemon

+  g_return_if_fail (_tp_dbus_daemon_is_the_shared_one (
+      tp_proxy_get_dbus_daemon (manager)));
+  g_return_if_fail (starter_account_manager_proxy == NULL);

I think it might be kinder to people who eventually get this wrong to do

if (!_tp_dbus_daemon_is_the_shared_one (_tp_proxy_get_dbus_daemon (manager)))
  {
    CRITICAL ("'manager' must use the TpDBusDaemon returned by
tp_dbus_daemon_dup()");
    g_return_if_reached ();
  }

and similarly "may only be called once per process".

+ * This function can only be called before the first usage of
+ * tp_account_manager_dup(). It is useful for applications using a custom
+ * #TpSimpleClientFactory and want the default Account Manager to use it. See
+ * tp_account_manager_new_with_factory().

“This function may only be called before the first call to
tp_account_manager_dup(), and may not be called more than once. Applications
which use a custom #TpSimpleClientFactory and want the default
#TpAccountManager to use that factory should call this after calling
tp_account_manager_new_with_factory().” or something?

• TpAccountChannelRequest: do not create a TpAccountManager for tmp handler

Looks fine.

• TpAccountManager is the "toplevel" object, do not create it through a factory

Looks fine.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.