[Bug 32053] Add a TpContactSearch proxy object

[bugzilla-daemon at freedesktop.org]

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

--- Comment #16 from Emilio Pozuelo Monfort <pochu27 at gmail.com> 2010-12-12 17:00:42 PST ---
(In reply to comment #15)
> tp_contact_search_new_finish should have (transfer full) and have
> G_GNUC_WARN_UNUSED_RESULT

fixed

> tp_contact_search_reset_async() looks a bit weird to me but I guess that's the
> price we have to pay if we want to be able to change the server without
> creating a new object.
> 
>  * Since: 0.13.9
> Lie! use 0.13.UNRELEASED

fixed

> Shouldn't tp_contact_search_search() stop a previous search, if any?
> You should pass self as weak_object to you call_ methods.

Not sure if that's necessary... since we require that subsequent searches call
_reset first, and _reset closes the search channel (and creates a new one)

> The TpAccount, server and limit should be proper GObject properties.
> Also, tp_contact_search_new_async() should be a tin wrapper around
> g_async_initable_new_async (ie, properties should be set by the GObeject
> property
> mechanism).

done

> TpAccountChannelRequest is leaked. You can unref it just after you called
> tp_account_channel_request_create_and_handle_channel_async() so it will be
> destroyed when it's done.

fixed

> _create_search_channel_cb: the error is leaked in the first error case.

oops, fixed

> _create_search_channel_cb should complete the operation synchronously not in
> idle.

not sure why, but done

> self->priv->keys is set in 2 places; is that expected?

ahem, no

> Also, why not return self->priv->keys in tp_contact_search_reset_finish() ?

done

> No API to ask for more results?

I haven't added it... is there consensus that we want this? it may make sense
since there's no way to request that, though I don't see simple UIs using it

> Like Smcv, I'd prefer to have an higher level object representing search
> result.

I haven't done that yet... I can look at it in the morning

> Niptick
> -------
> 
> We usually use : "GCallback _padding[7];" as padding.

done

>   if (error)
> We use if (ptr != NULL)

I like more the other syntax :) but I'll get used to the tp style

>   return tp_asv_get_strv (tp_channel_borrow_immutable_properties
> (self->priv->channel),
> this line is too long (and some others too actually).

all fixed

> tp_contact_search_init: No need to init to NULL/0, GObject does it for us.

ah ok

> I tend to use g_hash_table_unref instead of _destroy.

sounds good, changed

> close_search_channel: you can use tp_clear_object()

done

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