[Bug 75204] [next] remove TpAsv from public API

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Wed Feb 19 08:22:41 PST 2014


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

--- Comment #6 from Xavier Claessens <xclaesse at gmail.com> ---
I did not review the whole branch, just wondering what would be the best API
for TpAccountChannelRequest, and probably other similar helpers like
TpAccountRequest.

I was expecting TpAccountChannelRequest to replace its internal GHashTable by a
GVariantDict. What Guillaume's branch does is:
1) Create a GVariantDict for the request (which is basically a
GHashTable<string, GVariant>).
2) Create a GVariant from the GVariantDict
3) create a TpAccountChannelRequest which converts the GVariant into a
GHashTable<string, GVariant>

So that means a double conversion. I don't think performance really matters
here, so I wouldn't object, but maybe we can go one step further?

What I would suggest:

 - TpAccountChannelRequest->priv->request should be a GVariantDict.

 - tp_account_channel_request_new()'s request arg should be a GVariantDict that
we simply ref to become priv->request. Or even better: remove that arg and
caller can just first create an empty TpAccountChannelRequest and then call
_set_request_property().

 - _set_request_property() becomes a trivial g_variant_dict_insert_value
(self->priv->request, key, variant);

 - We convert the internal GVariantDict to a GVariant only when we are actually
doing the channel request. At that point the TpAccountChannelRequest becomes
invalide and shouldn't be used anymore. That's already the case I think but our
API doesn't enforce it.

 - Similar change for the hints

 - There is one issue with tp_account_channel_request_dup_request() because
that means we would have to call g_variant_dict_end() to return the GVariant
but that invalidates it... But that function is only used for unit tests, I see
no other reason to need it. Asked Ryan and he said we could add a
g_variant_dict_get_value(), or g_variant_new_dict(), but he's not really
enthousiast with it neither.

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