Bug 54917 - Should have GVariant helpers similar to tp_asv_
Summary: Should have GVariant helpers similar to tp_asv_
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/xc...
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-09-14 12:15 UTC by Xavier Claessens
Modified: 2012-09-15 13:45 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Comment 1 Guillaume Desmottes 2012-09-14 12:43:51 UTC
http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=variant&id=cb2fbab3b4079843e240035ebbf7c4f318e36f06

 * These functions provide convenient access to the values in such
+ * a mapping.

"in such mapping" ?

+ * Since: 0.UNRELEASED

These API are not new so maybe we should remove this.

http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=variant&id=ca47ad360751210e016632bbc8a8e50bc4aaace8

+GVariantClass
+tp_variant_type_classify (const GVariantType *type)
+{
+  return *g_variant_type_peek_string (type);

This is worth a comment and I'd use [0] (easier to understand).

+
+  switch (g_variant_classify (variant))
+    {
+    case G_VARIANT_CLASS_DOUBLE:

'case' should be idented.

Same for the others switch.

http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=variant&id=c6dc12c3f6b64eb5f526ccf775df5b399ec1d4a7

Can't you link on the internal lib and use _tp_asv_to_vardict() ?
Comment 2 Simon McVittie 2012-09-14 14:05:33 UTC
(In reply to comment #1)
>  * These functions provide convenient access to the values in such
> + * a mapping.
> 
> "in such mapping" ?

No, the patch is correct here. "in such mappings" would also be right.
Comment 3 Xavier Claessens 2012-09-15 13:44:50 UTC
(In reply to comment #1)
> http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=variant&id=cb2fbab3b4079843e240035ebbf7c4f318e36f06
> + * Since: 0.UNRELEASED
> 
> These API are not new so maybe we should remove this.

Only the internal functions are not new, all the public stuff are new.
 
> http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=variant&id=ca47ad360751210e016632bbc8a8e50bc4aaace8
> 
> +GVariantClass
> +tp_variant_type_classify (const GVariantType *type)
> +{
> +  return *g_variant_type_peek_string (type);
> 
> This is worth a comment and I'd use [0] (easier to understand).

done

> +
> +  switch (g_variant_classify (variant))
> +    {
> +    case G_VARIANT_CLASS_DOUBLE:
> 
> 'case' should be idented.
> 
> Same for the others switch.

done

> http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=variant&id=c6dc12c3f6b64eb5f526ccf775df5b399ec1d4a7
> 
> Can't you link on the internal lib and use _tp_asv_to_vardict() ?

Actually I'm wondering why we don't do that for all tests... done.

All fixed and merged.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.