[Bug 50341] Special trick to add a contact in WLM

[bugzilla-daemon at freedesktop.org]

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

--- Comment #12 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2012-05-28 11:36:59 PDT ---
Comment on attachment 62174
  --> https://bugs.freedesktop.org/attachment.cgi?id=62174
WIP: add tp_handle_ensure_async()

Review of attachment 62174:
 --> (https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=50341&attachment=62174)
-----------------------------------------------------------------

::: telepathy-glib/base-connection.c
@@ +2587,4 @@
>        goto out;
>      }
>  
> +  /* FIXME: Quick And Dirty hack */

I assume this is just because you can't be bothered doing a queueing system
right now, right?

::: telepathy-glib/handle-repo.c
@@ +41,2 @@
>  
> +G_DEFINE_INTERFACE (TpHandleRepoIface, tp_handle_repo_iface, G_TYPE_OBJECT);

You should put this in another patch; it just clutters up this one.

@@ +312,5 @@
> + * @callback: a callback to call when the operation finishes
> + * @user_data: data to pass to @callback
> + *
> + * Asyncronously normalize an identifier and create an handle for it. This could
> + * involve server round-trip. This is to be prefered over tp_handle_ensure() for

*a* server round-trip

"This is preferred". Actually shouldn't this be something like "This should be
used instead of tp_handle_ensure() for user provided contact identifiers, but
it is not necessary for identifiers from the server" ?

@@ +314,5 @@
> + *
> + * Asyncronously normalize an identifier and create an handle for it. This could
> + * involve server round-trip. This is to be prefered over tp_handle_ensure() for
> + * user provided contact identifiers, but not necessary for identifiers coming
> + * from server.

from *the* server

@@ +326,5 @@
> +    gpointer context,
> +    GAsyncReadyCallback callback,
> +    gpointer user_data)
> +{
> +  return TP_HANDLE_REPO_IFACE_GET_CLASS (self)->ensure_handle_async (self,

We should probably check this function is implemented and fallback somehow if
not? Oh, or does your default implementation below guarantee this is always
non-NULL?

@@ +347,5 @@
> +tp_handle_ensure_finish (TpHandleRepoIface *self,
> +    GAsyncResult *result,
> +    GError **error)
> +{
> +  return TP_HANDLE_REPO_IFACE_GET_CLASS (self)->ensure_handle_finish (self,

Same thing about checking here.

@@ +392,5 @@
> + *
> + * Since: 0.UNRELEASED
> + */
> +void
> +tp_handle_repo_iface_implement_ensure_handle_async (TpHandleRepoIface *self,

I don't understand this? Why don't we just make handle repos override these in
their class inits?

@@ +519,5 @@
> +{
> +  GSimpleAsyncResult *simple = (GSimpleAsyncResult *) result;
> +
> +  g_return_val_if_fail (g_simple_async_result_is_valid (result,
> +      G_OBJECT (self), NULL), 0);

I was going to say "we can check the source tag here?" but this is so
implementations don't need to implement _finish, right? If so, yes, good.

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