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

[bugzilla-daemon at freedesktop.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|eitan.isaacson at collabora.co |andrunko at gmail.com
                   |.uk                         |

--- Comment #4 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2011-11-14 05:33:15 PST ---
> +  { "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.)

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

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

> +PROTOCOL_IFACE_ADDRESSING = \
> +   'org.freedesktop.Telepathy.Protocol.Interface.Addressing'

Nitpicking: could be PROTOCOL + '.Interface.Addressing', which would probably
make it fit on one line.

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