[Bug 27397] Need tp_account_set_avatar_{async,finish}

[bugzilla-daemon at freedesktop.org]

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


Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
  Status Whiteboard|review-, minor changes      |review?, minor changes,
                   |                            |patch




--- Comment #4 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk>  2010-04-01 07:16:10 PST ---
(In reply to comment #2)
> 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 "")

done.

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

So am I. I just based my implementation on set_nickname.
Fixed.

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

As you prefer. Done.

> > +  g_value_init (&value, TP_STRUCT_TYPE_AVATAR);
> 
> tp_value_array_build() would be clearer, I think. (Yes we have new API \o/ )

done.

> > +  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));

done.

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

done.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.