[Bug 32053] Add a TpContactSearch proxy object

[bugzilla-daemon at freedesktop.org]

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

--- Comment #33 from Will Thompson <will.thompson at collabora.co.uk> 2011-01-24 01:42:37 PST ---
Review of attachment 42202:
 --> (https://bugs.freedesktop.org/review?bug=32053&attachment=42202)

I had a skim over the API. It looks nice! There are some places where more
documentation might be nice—specifically, TpContactSearchResult is very
sparsely documented­—but this isn't a merge blocker. I found a few nits, which
might be nice to fix before merging, but again they're not crucial.

Code-wise, I trust the judgement of others’ past review. I did notice a bug in
_result_dispose(). I didn't really look all that closely at the code in
general, since it's already been reviewed.

::: telepathy-glib/capabilities.c
@@ +481,3 @@
+ * tp_capabilities_supports_contact_search:
+ * @self: a #TpCapabilities object
+ * @with_limit: (out): if not %NULL, used to return %TRUE if the @limit

I think gtk-doc will complain that this function has no @limit argument.

::: telepathy-glib/contact-search-result.c
@@ +38,3 @@
+ *
+ * #TpContactSearchResult objects represent results for
+ * #TpContactSearch.

it representS results.

@@ +85,3 @@
+  gchar *name = (gchar *) n;
+
+  return g_strcmp0 (field->field_name, name);

Do you really need to cast 'n'? If so, you should cast it to (const gchar *).

@@ +136,3 @@
+  g_free (self->priv->identifier);
+  g_list_free_full (self->priv->fields,
+      (GDestroyNotify) tp_contact_info_field_free);

This function will break if it's called more than once, as it technically can
be. You should NULL out both its fields after freeing them so that it won't
break.

::: telepathy-glib/contact-search.c
@@ +407,3 @@
+   * TpContactSearch:server:
+   *
+   * The search server. This is only supported by some protocols,

this comma should be a semi-colon.

@@ +428,3 @@
+   *
+   * The maximum number of results that the server should return.
+   * This is only supported by some protocols, use

ditto.

@@ +471,3 @@
+   *
+   * Emitted when search results are received. Note that this signal may
+   * be called multiple times for the same search.

emitted, not called.

@@ +679,3 @@
+ * The keys are vCard field names in lower case, except when
+ * they're one of the special cases from telepathy-spec like
+ * "tell;cell" or "x-n-given". See the Channel.Type.ContactSearch

I think this should be "tel;cell". It's been a while since I looked at this
interface.

You can include hyperlinks in gtk-doc. Why not make this reference a clickable
link to http://telepathy.freedesktop.org/spec/Channel_Type_Contact_Search.html
?

::: tests/dbus/contact-search.c
@@ +1,1 @@
+/* Tests of TpContactSearch and TpContactSearchResult

tests for, not of.

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