[Bug 13422] Add nice API for channel creation

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Tue Aug 3 18:58:30 CEST 2010


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

--- Comment #16 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-08-03 09:58:30 PDT ---
Please add account-channels.c to the whitelist in introspect.am.

> + * If the channel is properly ensured but goes to a different handler then the
> + * operation will fail with %TP_ERROR_NOT_YOURS as error.

"successfully ensured" would be better English. I think this would be better
still:

    If the channel already exists and is already being handled, or if a
    newly created channel is sent to a different handler, this operation
    will fail with the error %TP_ERROR_NOT_YOURS. The other handler
    will be notified that the channel was requested again, and can
    move its window to the foreground, if applicable.

> + * tp_account_ensure_and_handle_channel_finish:
...
> + * Returns: %TRUE if the channel was successful created and you are handling
> + * it, otherwise %FALSE

"successfully created" (also in the _create_ version).

I think this docstring should also remind the API user about NotYours (just the
first sentence of the above, perhaps).

In request_and_handle_channel_cb, if ctx->cancellable has already been
cancelled when you call g_cancellable_connect (in practice this means it was
cancelled while the Create/EnsureChannel call was in-flight), then you'll call
Cancel() followed by Proceed(). You can detect that by only calling Proceed if 
(ctx->cancellable == NULL || ctx->cancel_id != 0), I think?

> +      /* Object has ben properly removed so ChannelRequest succeeded */

"has been removed without error, so"

Why not do this, which has one less g_error_new?

  GError e = { domain, code, message };

  if (domain == TP_ERRORS && code == TP_DBUS_ERROR_OBJECT_REMOVED)
    {
      ...
    }
  ...
  request_ctx_fail (ctx, &e);

> +  DEBUG ("Proceed sucess; waiting for the channel to be handled");

"Proceed succeeded; waiting"

> +  /* Request succeed */

"Request succeeded"

> +      "TpGlibTempHandler", TRUE, handle_channels, ctx, NULL);

It's not all that temporary any more? I think I'd prefer
"TpGLibRequestAndHandle".

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



More information about the telepathy-bugs mailing list