[Bug 32692] Support Protocol.Interface.Addressing in Gabble

[bugzilla-daemon at freedesktop.org]

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

--- Comment #5 from Andre Moreira Magalhaes <andrunko at gmail.com> 2011-11-14 09:11:12 PST ---
(In reply to comment #4)
> > +  { "protocol",       GABBLE_DEBUG_PROTOCOL },
> ...
> > +  GABBLE_DEBUG_PROTOCOL      = 1 << 28,
> ...
> > +#define DEBUG_FLAG GABBLE_DEBUG_PROTOCOL
> ...
> > +#include "debug.h"
> 
> I don't see any calls to DEBUG(), so I don't think you need any of this?
> 
> (I think it's fine that there's no debug-spam - Protocol is simple enough not
> to need any debug-spam.)
Done

> > addressing_normalize_vcard_address
> 
> I think this is in the wrong order: when you add x-facebook-id you're going to
> have to re-order it. As I said to Eitan before, I think it should be
> (pseudocode):
> 
>     if (it's x-jabber)
>       {
>         do Jabber things
>       }
> /* later, you will add:
>     else if (it's x-facebook-id)
>       {
>         do Facebook things
>       }
>  */
>     else
>       {
>         error!
>       }
> 
> Likewise for the URI, except that the scheme == NULL case comes first, so
> "xmpp" is also an else-if.
Done

> > +      g_object_get (G_OBJECT (protocol), "immutable-properties", &props, NULL);
> 
> Nitpicking: we like one property per line, to emphasize the paired arguments:
> 
>     g_object_get (G_O (p),
>         "i-p", &props,
>         NULL);
Done

> > +PROTOCOL_IFACE_ADDRESSING = \
> > +   'org.freedesktop.Telepathy.Protocol.Interface.Addressing'
> 
> Nitpicking: could be PROTOCOL + '.Interface.Addressing', which would probably
> make it fit on one line.
Done

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