[Bug 28312] Make TpContact emit a presence-changed signal

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-06-11 06:16:08 PDT ---
Review of attachment 36205:
 --> (https://bugs.freedesktop.org/review?bug=28312&attachment=36205)

This mostly looks good; I'd like a few minor changes.

> From: mortenmj <morten.mjelva at gmail.com>

You might want to fix your git user.name setting and commit --amend this patch,
before it becomes part of our revision history :-)

::: telepathy-glib/contact.c
@@ +873,3 @@
+   * TpContact::presence-changed:
+   * @contact: A #TpContact
+   * @presence: The contact's presence type 

"The new value of #TpContact:presence-type"

That way, the API user can follow the hyperlink to find out exactly how it
works.

@@ +874,3 @@
+   * @contact: A #TpContact
+   * @presence: The contact's presence type 
+   * @status: Protocol-defined description of the presence type

The new value of #TpContact:presence-status

@@ +875,3 @@
+   * @presence: The contact's presence type 
+   * @status: Protocol-defined description of the presence type
+   * @status_message: User-defined status message

The new value of #TpContact:presence-message

@@ +877,3 @@
+   * @status_message: User-defined status message
+   *
+   * Emitted when PresenceChanged is received for this contact.

I'd prefer "Emitted when this contact's presence changes"; then it's meaningful
even if you haven't read the Telepathy D-Bus API recently.

@@ +1662,2 @@
+// Presence properties might have changed. 
+// Set new values, and emit the TpContact::presence-changed signal.

/* */, please; we don't use C++ comments in Telepathy C code.

I don't think this comment actually adds much value, though, so I'd be inclined
to just delete it.

@@ +1677,2 @@
+  tp_value_array_unpack (presence, presence->n_values, &type, &status,
+      &message);

Use a literal "3" for the second argument; it's used as a sanity check (to be
able to warn/abort the number of arguments that follow it doesn't match
presence->n_values), and using presence->n_values defeats that.

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