Bug 55024

Stream tube parameters would be nice too (they're basically the same).

Created attachment 67287 [details] [review]
TpDBusTube:  obtain parameters as vardict

Comment on attachment 67287 [details] [review]
TpDBusTube:  obtain parameters as vardict

Review of attachment 67287 [details] [review]:
-----------------------------------------------------------------

This looks great. Want to add the stream-tube equivalent while you're here? :-)

A few minor documentation things, which I'll just fix myself while I'm committing it:

::: telepathy-glib/dbus-tube-channel.c
@@ +517,5 @@
> + * @self: a #TpDBusTubeChannel
> + *
> + * Return the parameters of the dbus-tube channel in a variant of
> + * type %G_VARIANT_TYPE_VARDICT whose keys are strings representing 
> + * parameter names and values are GValues representing corresponding 

... and values are variants representing ...

@@ +525,5 @@
> + * yet been offered or the parameters property has not been set.
> + *
> + * Use g_variant_lookup() or g_variant_lookup_value() for convenient
> + * access to the values, the expected types of which should match the ones
> + * defined upon the creation of the parameters #GHashTable.

The second clause "the expected types..." looks like copy/paste damage, it's not really particularly relevant here. I'd replace this sentence with something like:

    Use g_variant_lookup(), g_variant_lookup_value() or functions like tp_vardict_get_uint32() for convenient access to the values.

(tp_vardict_get_uint32 is new in master.)

@@ +527,5 @@
> + * Use g_variant_lookup() or g_variant_lookup_value() for convenient
> + * access to the values, the expected types of which should match the ones
> + * defined upon the creation of the parameters #GHashTable.
> + *
> + * Since: 0.20.0

We say "Since: 0.UNRELEASED" in patches, and whoever does the release greps for UNRELEASED and fixes the reference. Otherwise, if your patch didn't get applied until 0.21.5 or something, it'd be inaccurate.

Thanks, committed for 0.19.10 with the documentation adjustments I mentioned.

For future reference, it also needed a Returns: line:

 * Returns: (transfer full): a new reference to a #GVariant

and please avoid trailing whitespace in future (git should generally warn you about that, I think?)

Retitling to what's left, feel free to assign to yourself if you intend to work on this.

Created attachment 67388 [details] [review]
TpStreamTubeChannel: add parameters-vardict and its  getter

---

Much like Chandni did for TpDBusTubeChannel, but with the addition of a "parameters-vardict" property.

Created attachment 67389 [details] [review]
TpDBusTubeChannel: add parameters-vardict

I read the patch and it looks fine to me. I didn't apply or did a make check though. I think this patch is important to make this bug completely fixed.

Created attachment 67622 [details] [review]
Add ':' to tp_dbus_tube_channel_dup_parameters_vardict comments

I checked out the applied patch from the master again and found that my build was failing at a missing ':' so attaching a correction.

Comment on attachment 67622 [details] [review]
Add ':' to tp_dbus_tube_channel_dup_parameters_vardict comments

(In reply to comment #9)
> I checked out the applied patch from the master again and found that my
> build was failing at a missing ':' so attaching a correction.

Travis already fixed this in <http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=1bbda3bc>.

Oh, then it's fine! I probably missed that commit due to insufficient commit message.

Comment on attachment 67388 [details] [review]
TpStreamTubeChannel: add parameters-vardict and its  getter

Review of attachment 67388 [details] [review]:
-----------------------------------------------------------------

++

Comment on attachment 67389 [details] [review]
TpDBusTubeChannel: add parameters-vardict

Review of attachment 67389 [details] [review]:
-----------------------------------------------------------------

++

Merged to master for 0.19.10 (or 0.20.0 depending on which one we release).