[Bug 31686] Add avatar support to TpBaseProtocol

[bugzilla-daemon at freedesktop.org]

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

--- Comment #2 from Eitan Isaacson <eitan.isaacson at collabora.co.uk> 2010-11-18 15:44:00 PST ---
(In reply to comment #1)
> > +    <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.)
> 

I get confused as to how/when this gets changed.
8dd94a3

> > + * @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"?
> 

I do. e4db32d

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

Good point! 34b8057

> 
> > +    PPA_SUPPORTED_AVATAR_MIME_TYPES,
> 
> Surely ProtocolAvatarProp abbreviates to PAP, not PPA?
> 

Um, yeah. e879ad7

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

Sure. The other virtual methods don't use the typedef. 3b15af9

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

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

3bbe64d

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