[Bug 27397] Need tp_account_set_avatar_{async,finish}

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=27397


Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|telepathy-                  |guillaume.desmottes at collabor
                   |bugs at lists.freedesktop.org  |a.co.uk
          QAContact|                            |telepathy-
                   |                            |bugs at lists.freedesktop.org
  Status Whiteboard|review?                     |review-, minor changes




--- Comment #2 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2010-04-01 05:56:04 PST ---
(Please use the patch keyword to get things in my review queue, and assign
things you're actively working on to yourself.)

I'd like a better description of the parameters' constraints, enforced with
g_return_if_fail. I think these assertions would be appropriate:

* avatar != NULL || len == 0 (you can only use NULL if there's no data)
* mime_type != NULL (if you really, truly want it unspecified for some reason,
you can use "")

> +tp_account_set_avatar_async (TpAccount *account,

I generally prefer the first argument of a method (or at least, a thing that
would be a method if you were writing C++) to be called self; telepathy-glib
isn't 100% consistent on this though.

> +  GArray tmp = { (gchar *) avatar, len };

Now that GArray is refcounted and otherwise magical, I'm uncomfortable with
allocating it on the stack like this, at least in new code. Please allocate an
array, and (if len is nonzero) append the data to it.

> +  g_value_init (&value, TP_STRUCT_TYPE_AVATAR);

tp_value_array_build() would be clearer, I think. (Yes we have new API \o/ )

> +  tp_cli_dbus_properties_call_set (TP_PROXY (account), -1,

The first argument of TpProxy's tp_cli methods is gpointer, so you don't need
this cast.

> +      _tp_account_property_set_cb, result, NULL, G_OBJECT (account));

It's useless to pass @account as the weak object; tp_cli calls hold a ref to
the proxy anyway, so this weak ref will never die.


-- 
Configure bugmail: http://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.