[Bug 27511] Need API to represent capabilities

[bugzilla-daemon at freedesktop.org]

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

Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|review-                     |
           Keywords|                            |patch

--- Comment #5 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-04-14 01:54:26 PDT ---
(In reply to comment #4)
> > +tp_capabilities_is_specific_to_contact
> 
> Perhaps this would be better as some concept of "source" with three values?
> 
> * from the connection
> * for a contact, but guessed from the connection's caps
> * for a contact, from ContactCapabilities
> 
> and in future (Bug #20774) also:
> 
> * for a Protocol

I can't really think of a use case when user will end up with a TpCapabilities
without having requested from a TpConnection/TpContact/TpProtocol before; so
I'm not sure that's that useful.
Furthermore, that'd introduce some kind of two-way relationship between
classes.


> I'm not sure whether ...supports_text_chats() is the best name we can have for
> a function about 1-1 chats, but I can't immediately think of anything better.

I mimiced the name of the tp-qt4 method.

> > +static gboolean
> > +supports_text_channel (TpCapabilities *self,
> > +    TpHandleType exported_handle_type)
> 
> Do you mean "expected" rather than "exported"?

I do. Fixed.

> I think this helper function could usefully take the channel type as a
> parameter too, and be called supports_simple_channel() or something.

Done.

> > +      fixed =  g_value_get_boxed (g_value_array_get_nth (arr, 0));
> [add blank line]
> > +      if (g_hash_table_size (fixed) != 2)
> > +        continue;
> 
> I'd prefer a blank line where indicated, and the same for if (!valid).

Done.

> Tests
> =====
> 
> > +++ b/tests/capabilities.c
> 
> I'd prefer new tests to use GTest (g_test_add() etc.), and use the g_assert_foo
> family (particularly g_assert_cmpuint and friends) in preference to MYASSERT().

done.

> > +  g_value_init (&class, TP_STRUCT_TYPE_REQUESTABLE_CHANNEL_CLASS);
> 
> I'd prefer tp_value_array_build() rather than messing about with temporary
> GValues.

Good point; I forgot about it. Done.


I commited changes in separated commits for easier reviewing. I'll squash them
once you have looked at them.

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