On 11/05/10 13:36, Olli Salli wrote:
Finally had time to take a more thorough look at your code. Some observations:
Account::createStreamTube: about including TargetHandle == 0 in the
request when contact is NULL: the spec states about TargetHandle that
"If this is present in a channel request, it must be nonzero". So, I
think you should return a PendingChannelRequest which fails
immediately if the contact isn't valid. However, I realize what you
did is what the other Account methods are doing as well - Andre, can
you give me a heads up on why this is so? Should we just change them
all in fact?
I don't remember exactly why we used this here. But I remember Simon
explaining why we should do so. I will check with him and let you know.
Account::create*DBusTube: please comment them out as long as they
aren't actually implemented ;) (That is, if a merge is planned at this
point). Also, I think "SingleDBusTube" could be clearer as
"P2PDBusTube", as that's the terminology we've otherwise used.
TubeChannel: I think it indeed makes sense to not have a separate
TubeState feature (which you've commented out already), as any user of
a tube channel will be interested in the state. As a side note, the
warning output in tubeState() actually says FeatureChatState must be
requested ;) In fact, to have the separate feature would be in a sense
similar to having Connection's Status as an optional introspectable,
which is clearly wrong.
The documentation for FeatureCore says it's implicitly added to the
set of features for isReady() and becomeReady() - is this really true?
I see the FT channel docs say the same, but I don't think such a
mechanism actually exists (while it would be useful). However,
becomeReady does have a default parameter of Channel::FeatureCore, but
that doesn't extend to subclasses. Andre? Maybe we could make
ReadinessHelper always assume all registered introspectables with
critical == true as requested, or something?
When defining an introspectable you can define a FeatureX depending on
another FeatureY. What we do is to make all introspectables depend on
FeatureCore, so when they are asked to be ready FeatureCore will be
prepared first.
Also ReadyObject takes the "FeatureCore" as a param, so when calling
becomeReady with no params it will use the feature set as the core feature.
BR
Andre
_______________________________________________
telepathy mailing list
telepathy@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/telepathy
