[Bug 31686] Add avatar support to TpBaseProtocol

[bugzilla-daemon at freedesktop.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
  Status Whiteboard|                            |review-

--- Comment #1 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-11-18 06:23:11 PST ---
This is clearly good enough to not block undrafting; thanks.

I'd like a proof-of-concept branch for at least Gabble (the trivial case, where
avatar sizes are constant), and preferably also Haze (the non-trivial case,
where you can get avatar sizes from prpl_info - see connection-avatars.c).

> +    <tp:added version="0.19.14">(as stable API)</tp:added>

No, it wasn't.

(But this will conflict with the actual undraft when merged, and we can use the
correct one as the merge resolution; so, no big deal.)

> + * @max_height: (out): used to return the maximum height in pixels of an
> + *  avatar on this protocol, which may be 0
> + * @max_width: (out): used to return the maximum width in pixels of an avatar
> + *  on this protocol, which may be 0

I think you mean "... on this protocol, or 0 if there is no limit"?

> +  AvatarSpecs *avatar_specs;

Why a pointer? If this was just a struct, it'd automatically be allocated as
part of the private struct - less fragmentation. You'd access it like
"self->priv->avatar_specs.min_height".

> +    PPA_SUPPORTED_AVATAR_MIME_TYPES,

Surely ProtocolAvatarProp abbreviates to PAP, not PPA?

> +  void (*get_avatar_details) (TpBaseProtocol *self,

If you're going to make a typedef for documentation purposes (as you did here),
please use it - "TpBaseProtocolGetAvatarDetailsFunc get_avatar_details;"

>        gchar **vcard_field);
> 
> +  void (*get_avatar_details) (TpBaseProtocol *self,
[...]
> +      guint *max_bytes);
> +
>    const TpPresenceStatusSpec * (*get_statuses) (TpBaseProtocol *self);

Adding a member to a struct is an ABI break. You need to put get_avatar_details
after get_statuses, and reduce the number of spare callbacks by 1 at the same
time.

I'd personally also use "rec" instead of "recommended" in variable names, but
that's a matter of taste.

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