[Bug 21097] proxy subclasses should support optional features

[bugzilla-daemon at freedesktop.org]

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

--- Comment #8 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-04-06 08:48:54 PDT ---
(In reply to comment #7)
> +    FEATURE_STATE_INVALID = GPOINTER_TO_INT (NULL),
> Any reason to not use "= 0" ?

The code relies on GPOINTER_TO_INT (g_hash_table_lookup (ht, an_absent_key))
returning FEATURE_STATE_INVALID. In practice we (and most other C libraries)
will fail horribly on any platform where NULL isn't all-bits-zero, though.

> Is it sane to allow to call tp_proxy_prepare_async with a NULL callback?

TpAccount allows it, and it doesn't seem harmful. The use-case is fairly
tenuous (getting a head-start on preparing a feature that someone else will
later wait for).

> Shouldn't we have TP_CHANNEL_FEATURE_GROUP rather than including it with CORE?

In the current TpChannel code, new channels are "trying to prepare" the group
stuff, even if nobody asked for it. I'm inclined to think that that's an API
guarantee.

I suppose it might make sense to have GROUP as a separate feature so CORE
doesn't have to wait for it... but the current code will wait for both anyway,
because there's a single introspect pipeline.

> tp_channel_is_ready's doc:
> + * This is a less general form of tp_proxy_is_prepared(), which should be
> + * used in new code.
> should NOT be used. Or maybe I miss-parsed this comment?
> Same comment for TpChannel:channel-ready

You did mis-parse, but I agree it could be clearer. The intended meaning was
"This is a less general form of t_p_i_p(); t_p_i_p() should be used in new
code", but I think that's an unwieldy way to phrase it.

Perhaps "New code should use t_p_i_p(), which is a more general form of this
method"?

> I guess it's ok to call tp_cli_connection_call_connect on a not prepared
> TpConnection right?

Yes, the TpConnection already knows that it's a Connection.

> Shouldn't we deprecate old API?

Not yet, I don't think; deprecation is rather a big stick (it breaks all of our
development builds, because they have -Werror and -Wdeprecated-declarations),
and we probably shouldn't deprecate API for which the replacement requires a
bleeding-edge dependency. I'd be inclined to deprecate the call_when_ready
family in 0.13.0.

It's an unfortunate situation... I'd like to have a way to get non-fatal
warnings for things that aren't yet fully deprecated, but are on the way there
(like tp_get_bus(), premature deprecation of which broke the buildbot today).

> Probably a bit out of scope for this branch, but as an API user I have to say
> that I'd really love to have a way to ask "prepare this object and the objects
> it's owning as well".

Yes, we should have that; yes, it's out of scope; and no, I can't think of a
nice way to express "prepare all of this AM's accounts, with the AVATAR
feature".

I think this should be done case-by-case rather than as a general thing: it
makes sense to want to prepare all of an AM's Accounts, but I don't think it
makes sense to want to prepare all of a Connection's Channels.

Plural operations that might be worth having a shorthand feature later:

* given the AccountManager, prepare all valid Accounts
* given the AccountManager, prepare all valid and invalid Accounts
* given a ChannelDispatchOperation, prepare the Account, the Connection and all
of the Channels
* given a ChannelDispatcher with the OperationList interface, prepare all the
CDOs
* given an Account, prepare the ConnectionManager and the (future) Protocol
* given a ChannelRequest, prepare the Account

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