[Bug 30310] Make GetContactAttributes public for re-use

[bugzilla-daemon at freedesktop.org]

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

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
  Status Whiteboard|                            |review-, minor changes
         AssignedTo|telepathy-bugs at lists.freede |eitan.isaacson at collabora.co
                   |sktop.org                   |.uk

--- Comment #3 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-09-21 10:55:12 PDT ---
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.

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

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

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.

> 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

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

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

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

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