[Bug 13422] Add nice API for channel creation

[bugzilla-daemon at freedesktop.org]

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

--- Comment #18 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-08-04 01:40:00 PDT ---
(In reply to comment #16)
> Please add account-channels.c to the whitelist in introspect.am.

done.

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

done.

> > + * 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).

fixed.

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

done.

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

I just added an early return if g_cancellable_is_cancelled() returns TRUE.

> > +      /* Object has ben properly removed so ChannelRequest succeeded */
> 
> "has been removed without error, so"

fixed.

> 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);

Good point; done.

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

fixed.

> > +  /* Request succeed */
> 
> "Request succeeded"

fixed.

> > +      "TpGlibTempHandler", TRUE, handle_channels, ctx, NULL);
> 
> It's not all that temporary any more? I think I'd prefer
> "TpGLibRequestAndHandle".

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.