[Bug 24652] TpContact should support Location

[bugzilla-daemon at freedesktop.org]

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

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 |guillaume.desmottes at collabo
                   |sktop.org                   |ra.co.uk
          QAContact|                            |telepathy-bugs at lists.freede
                   |                            |sktop.org

--- Comment #3 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-04-05 06:18:41 PDT ---
In general this looks good. Some minor changes:

> + * Return the contact's user-defined location or NULL it he didn't define one.

"%NULL if the location is unspecified", to avoid the he / he/she / they
problem?

> + * Returns: the same #GHashTable (or NULL) as the #TpContact:location property

%NULL

> +   * If this contact has set a user-defined location, a string to
> +   * #GValue * hash table containing his location. If not, %NULL.

I'd add something like "tp_asv_get_string() and similar functions can be used
to access the contents."

> +          tp_cli_connection_interface_location_call_get_locations (

I don't think this slow-path is actually needed. The Contacts interface was
made mandatory in 0.17.23, and Location wasn't added until 0.17.27, so I'd be
inclined to say that if your CM implements Location, but not Contacts, then you
don't deserve geolocation :-)

Notes for future contributions:

> +contact_maybe_set_location (TpContact *contact,

I loosely prefer the first argument to be called self.

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