[Bug 39189] [next] decide on a policy for transfer, naming and containers

[bugzilla-daemon at freedesktop.org]

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

--- Comment #22 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2012-09-04 11:30:31 UTC ---
(In reply to comment #16)
> Here is already a branch to remove _borrow_:
> http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=transfer

Continuing my review.

General comment: when what we're doing conceptually is renaming a function +
providing backwards compat, I generally prefer the diff to look as though
that's what we're doing, rather than introducing a new function with the new
name and a copy of the old implementation:

 /**
- * borrow_thing:
+ * get_thing:
  *
  * blah blah blah do things
  */
 gchar *
-borrow_thing (...)
+get_thing (...)
 {
   return implementation ();
 }
+
+/**
+ * borrow_thing:
+ *
+ * An old name for get_thing().
+ *
+ * Deprecated: blah blah blah
+ */
+ gchar *
+ borrow_thing (...)
+ {
+   return get_thing (...);
+ }

In other words, I like the way you did tp_proxy_get_interface_by_id().

When the implementation is as trivial as "return self->priv->thing" it doesn't
particularly matter, though.

----------------------------

-/* FIXME: in Telepathy 1.0, rename to tp_protocol_get_param */
 /**
  * tp_protocol_dup_param:
  * @self: a protocol
@@ -939,6 +938,7 @@ G_GNUC_END_IGNORE_DEPRECATIONS
  * or %NULL if not supported. Free with tp_connection_manager_param_free()
  *
  * Since: 0.17.6
+ * Deprecated: Since 0.UNRELEASED. Use tp_protocol_get_param() instead.
  */

No, please don't deprecate this one: just remove the comment. dup_param() is
useful in its own right, because get_param() returns something that loses
validity when the TpProtocol is freed.

  * a list of #TpConnectionManagerParam structures, owned by the caller
  *
  * Since: 0.17.6
+ * Deprecated: Since 0.UNRELEASED. Use tp_protocol_get_param() instead.
  */
 GList *
 tp_protocol_dup_params (TpProtocol *self)

Er, why are you deprecating this? If you want to list all the parameters (e.g.
for Empathy's fallback account creation UI), tp_protocol_dup_params() is nicer
than tp_protocol_dup_param_names() followed by repeated
tp_protocol_(get|dup)_param().

+DBusGProxy *
+tp_proxy_get_interface_by_id (TpProxy *self,
+ GQuark iface,
+ GError **error)
+{

The indentation here is weird (although copying from cgit to here destroyed the
indentation anyway).

+++ b/tools/glib-client-gen.py
...
- self.b(' iface = tp_proxy_borrow_interface_by_id (')
+ self.b(' iface = tp_proxy_get_interface_by_id (')

glib-client-gen is meant to generate backwards-compatible code by default, so
that it's safe to drop it into an old project and not bump the telepathy-glib
dependency. I would prefer:

    if self.tp_proxy_api >= (0, 19, 9):
        self.b(' iface = tp_proxy_get_interface_by_id (')
    else:
        self.b(' iface = tp_proxy_borrow_interface_by_id (')

and patching each Makefile.am that calls it to use --tp-proxy-api=0.19.9 (or
whatever the next version of telepathy-glib is going to be - I've lost track).

Or, if you don't think this is actually useful, I would accept a patch that
deleted the tp_proxy_api tests and always assumed the latest.

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