[Bug 32053] Add a TpContactSearch proxy object

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Tue Jan 18 11:14:23 CET 2011


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

--- Comment #26 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2011-01-18 02:14:21 PST ---
/**
 * TpContactSearchResult:
 *
 * A proxy object for the results of a Telepathy contact

It's not a proxy (in the sense of a TpProxy).
Same for TpContactSearch.


struct _TpContactSearchResultPrivate
  GList *fields;
Please add a comment explaining the content of the list and if it's owned or
borrowed.


     case PROP_IDENTIFIER:
        g_free (self->priv->identifier);
As it's construct only, we ususuall don't free and use:
g_assert (self->priv->identifier == NULL);  /* construct-only */


tp_contact_search_result_get_fields
Please document that the returned list should be free using g_list_free()


tests/contact-search-result.c
is leaky, use check-valgrind to check for leaks.


* They also abstract the creation of contact search channels, but you need
 * to call @tp_contact_search_reset_async between searches.
This isn't very clear imho. I'd focus on explaining how the object should be
used; channel creation is a TP implementation detail.

Use DEBUG() instead of g_print().


  request = tp_asv_new (
      TP_PROP_CHANNEL_CHANNEL_TYPE,
      G_TYPE_STRING,
      TP_IFACE_CHANNEL_TYPE_CONTACT_SEARCH,
      NULL);
  if (self->priv->server != NULL)
    tp_asv_set_string (request,
        TP_PROP_CHANNEL_TYPE_CONTACT_SEARCH_SERVER,
        self->priv->server);
  if (self->priv->limit != 0)
    tp_asv_set_uint32 (request,
      TP_PROP_CHANNEL_TYPE_CONTACT_SEARCH_LIMIT,
      self->priv->limit);
Add some \n between those.


        self->priv->account = g_object_ref (g_value_get_object (value));
Use g_value_dup_object()

account and server params are contruct only so please add an assertion as
explained above.


   * use @tp_capabilities_supports_contact_search to check if it's
syntax is "use tp_capabilities_supports_contact_search() to check.."
Same for other doc comments reffering to functions.



      g_cancellable_cancel (self->priv->cancellable);

      while (self->priv->async_res != NULL)
        g_main_context_iteration (NULL, TRUE);
This is a bit weird; maybe we should request a g_cancellable_cancel_async() in
glib?
Do we really need to wait btw ?


tp_contact_search_start(): shouldn't we check if another search is already
active?


+  if (with_limit)
if (with_limit != NULL)

+      if (!tp_strdiff (chan_type, TP_IFACE_CHANNEL_TYPE_CONTACT_SEARCH))
use "continue" to limit nesting.

tp_capabilities_supports_contact_search: you can use tp_value_array_unpack()


Having test for TpContactSearch would make me an happier badger...

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



More information about the telepathy-bugs mailing list