[Bug 55104] Provide GVariant-based access to TpCallChannel:state-details

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Mar 3 08:41:15 PST 2014


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

--- Comment #3 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
Comment on attachment 94833
  --> https://bugs.freedesktop.org/attachment.cgi?id=94833
turn TpCallChannel:state-details to a GVariant

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

::: telepathy-glib/call-channel.c
@@ +409,5 @@
>  
>    self->priv->state = state;
>    self->priv->flags = flags;
>    self->priv->state_reason = _tp_call_state_reason_new (reason);
> +  self->priv->state_details = tp_asv_to_vardict (details);

Floating ref needs sinking now

@@ +594,4 @@
>        "CallState", NULL);
>    self->priv->flags = tp_asv_get_uint32 (properties,
>        "CallFlags", NULL);
> +  self->priv->state_details = tp_asv_to_vardict (tp_asv_get_boxed (properties,

Also here

@@ +905,5 @@
>    /**
>     * TpCallChannel:state-details:
>     *
> +   * Detailed information about #TpCallChannel:state. It is a #GVariant
> +   * if type #G_VARIANT_TYPE_VARDICT.

s/if/of/ (but at least you fixed "infoermation")

@@ +1092,5 @@
>     * @state: the new #TpCallState
>     * @flags: the new #TpCallFlags
>     * @reason: the #TpCallStateReason for the change
> +   * @details: additional details as a
> +   *   #GVariant of type #G_VARIANT_TYPE_VARDICT readable.

I think the word "readable" is redundant here. It was part of the hint pointing
users towards the tp_asv_* family.

@@ +1208,4 @@
>   * @self: a #TpCallChannel
>   * @flags: (out) (allow-none) (transfer none): a place to set the value of
>   *  #TpCallChannel:flags
> + * @details: (out) (allow-none) (transfer full): a place to set the value of

It seems inconsistent that this is (transfer full) but the TpCallStateReason is
(transfer none). The _get_ naming hints API users towards (transfer none), I
think.

Any thoughts on whether to squash the variant, or maybe even the state and
flags too, into the TpCallStateReason (with a rename to ...StateDetails or
something if it gains the state and flags)?

I would like to at least add some ABI padding to the TpCallStateReason.

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