[Bug 38142] proxy factories

[bugzilla-daemon at freedesktop.org]

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

--- Comment #48 from Xavier Claessens <xclaesse at gmail.com> 2011-07-15 03:01:01 PDT ---
(In reply to comment #47)
> You should add in TpSimpleClientFactory's doc which features are asked to be
> prepared by default.

Fixed

>  * @create_account_manager: virtual method used to create a #TpAccountManager;
>  *  see tp_simple_client_factory_dup_account_manager()
> tp_simple_client_factory_dup_account_manager() doesn't exist any more.
> 
> Ditto for the others.

Fixed

> static TpConnection *
> create_connection_impl (TpSimpleClientFactory *self,
>     const gchar *object_path,
>     GError **error)
> 
> Shoudln't we do something like:
> 
> static TpConnection *
> create_connection_impl (TpSimpleClientFactory *self,
>     const gchar *bus_name,
>     const gchar *object_path,
>     GError **error)
> 
> to stay coherent with tp_connection_new and allows user to create TpConnection
> from their bus name rather than their path?

There is not a single usage of that in the whole tp-glib, I've verified. IMO
that's just making stuff more complicate for no benefit. And it is more
coherent with other objects (TpAccount, TpChannel) which takes only an
object-path. IMO constructing a TpConnection from a bus_name is just legacy,
the spec always give the object path.

> I do beliver that oa{sv} (object-path and immutable properties) is the right
> way to advertise objects and that we should use it everywhere in
> Telepathy. As we are "about" to break the D-Bus API maybe we could add a
> "GHashTable *immutable_propertiers" argument to all of these methods to be
> future proof?

IMO that would be too early generalization. The spec does not provide
immutable-properties table for those objects, so user would wonder why we have
that currently useless arg. But we could keep this in mind for future API break
if needed.

> A few lines are still too long, you should configure your editor to mark them
> in red or something:
>   tp_clear_pointer (&self->priv->desired_account_manager_features,
> g_array_unref);
>   connection = TP_SIMPLE_CLIENT_FACTORY_GET_CLASS (self)->create_connection
> (self,
> tp_simple_client_factory_ensure_channel_dispatch_operation
> (TpSimpleClientFactory *self,

I do have a 80 col lines, but I'm not that strict on the limit usually :p
Fixed.

-- 
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.