[Bug 30310] Make GetContactAttributes public for re-use

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Eitan Isaacson <eitan.isaacson at collabora.co.uk> 2010-09-22 01:03:54 PDT ---
(In reply to comment #3)
> My long-running contact-lists branch has a more hackish version of this as
> library-internal API, but yours seems sufficiently good to be real API, which
> is nice.
> 
> > 266 tp_contacts_mixin_get_contacts_attributes (GObject *obj,
> 
> contact_attributes not contacts_attributes, please.
> 

6861220

> It should also g_return_val_if_fail that obj is a TpBaseConnection with a
> TpContactsMixin.
> 

ba4d915
My brain stopped working, it's too late here. Not sure if I am checking for the
mixin correctly. Looked like the most straightforward way.

> Please decide on a policy for how this function deals with disconnected
> connections, and document it. Possibilities are:
> 
> - raise a GError and document that it's OK to call it disconnected, but it will
> fail
> - g_return_val_if_fail and document that the connection must still be connected
> 
> If you make it raise a GError, you could also not ignore the error from
> tp_handles_client_hold (which can't happen when telepathy-glib calls it, but
> could happen from another caller).

Let's just assume it's connected.
72ac056

> 
> I think it's worth having an extra, nullable assumed_interface argument, which
> ContactList and Addressing can use to avoid having to copy their
> caller-supplied interfaces list in order to always include the relevant
> interface.

ccab5f4

> 
> > And non-valid handles will be
> 
> Any invalid handles will be
> 
> > + * will be held by this client.
> 
> "held on behalf of this client, as if via tp_handles_client_hold()" would be
> better?
> 
> > + * Provide attributs for all requested
> 
> attributes
> 

92707b9

> (In reply to comment #1)
> > Probably need a docstring there. Coming soon.
> 
> ... where by "probably need" you mean "to pass 'make check && make distcheck',
> it will need"; it'll also need adding to the -sections.txt to pass checks. In
> general, every public API function should be documented (and added to
> -sections.txt) by the same patch that introduces it.
> 

e10fad6

I'll squash all these changes into one patch with a good commit message before
merging.

> > +tp_contacts_mixin_get_contacts_attributes (GObject *obj,
> > +    const GArray *handles, const gchar **interfaces, const gchar *sender)
> 
> One argument per line, please (I know the existing code had this wrong).
> 

ccab5f4

> The newly public method either needs the (skip) annotation, or correct
> GObject-Introspection annotations, or both. I'd be inclined to (skip) it as not
> useful for introspection, but annotations are often useful documentation
> anyway. I think it's something like:
> 
>     @handles: (element-type guint): ...
>     @interfaces: (array zero-terminated=1): ...
>     @sender: (allow-none): ...
> 
>     Returns: (element-type guint GLib.HashTable): ...

e10fad6

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