[Bug 38142] proxy factories

[bugzilla-daemon at freedesktop.org]

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

--- Comment #55 from Xavier Claessens <xclaesse at gmail.com> 2011-07-15 10:22:34 PDT ---
Thanks for that contribution :)

(In reply to comment #54)
> +++ b/telepathy-glib/automatic-client-factory-internal.h
> @@ -0,0 +1,45 @@
> +/*
> + * Automatic client factory internal
> 
> Gee, what a helpful description of the file. ;)

suggestion?

> I rearranged the arguments of these three functions to put the GError ** last,
> and put the factory first. They're methods on the factory, no?

Looks better indeed. You should do the same for those defined in
simple-client-factory-internal.h too :)

> + * SECTION:automatic-client-factory
> + * @title: TpAutomaticClientFactory
> + * @short_description: factory creating higher level objects
> 
> That description tells me even less than the class name. “Factory for
> specialized TpChannel subclasses” sounds better. I've changed this.

Atm it does specialized subclasses only for TpChannel but more could be added
later. Guess we can change the doc when/if we actually do more than channels.

> +#define chainup ((TpSimpleClientFactoryClass *) \
> +    tp_automatic_client_factory_parent_class)
> 
> This is a new one, I like it in theory—but I think I would have used
> 
>   static TpSimpleClientFactoryClass *chainup = NULL;
> 
>   ... class_init {
>     ...
>     chainup = (TpSimpleClientFactoryClass *)
> tp_automatic_client_factory_parent_class;
>   }
> 
> Maybe that's just me.

I find my idea simpler, tbh. Since it is static casting anyway, there is no
real benefit in keeping a ptr to the parent class.

> /* FIXME: This is temporary solution to share TpChannel objects until
>  * TpSimpleClientFactory can be used for that */
> void
> _tp_channel_dispatch_operation_ensure_channels (TpChannelDispatchOperation
> *self,
>     GPtrArray *channels)
> 
> Why can't it be used for that now? How temporary is temporary? What would
> actually need to be done to fix this FIXME?

because this branch is a subset of what I have in contact-list branch. I wanted
to get this part already merged and keep using the old channel factory for now.

> TpAutomaticClientFactory doesn't seem to show up in the generated documentation
> for me. At least some of these classes should have code samples for how to
> actually used them. I'm lost in a sea of very similarly-named classes whose
> documentation describes them as “Factory creating higher level objects” or
> “Factory creating higher level proxy objects”.

TpBasicProxyFactory, TpAutomaticProxyFactory and TpClientChannelFactory will
all be deprecated soon. TpSimpleClientFactory and TpAutomaticClientFactory
replaces them.

> I have pushed
> http://cgit.collabora.com/git/user/wjt/telepathy-glib.git/log/?h=factories
> which fixes some documentation nits—plus a build failure and g_object_unref
> (NULL) which I found within 30 seconds of cloning the branch ;).

Thanks, already cherry-picked one commit that can go to master right away.

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