Bug 31467

wjt wrote:

Can the MC implementation branch also bin the property and code from
Account.Interface.Compat that it replaces?


+  if (schemes == NULL)
+    {
+      gchar *empty[] = { NULL };
+      schemes = empty;
+    }

Forgive my C ignorance: is dereferencing 'schemes' actually kosher once we
leave the if {} block? Wouldn't it be more obvious to just not run the
following loop...

+  for(scheme = schemes[pos++]; scheme != NULL; scheme = schemes[pos++])
+    {
+      if (!tp_strdiff (scheme, uri_scheme))
+        old_association = TRUE;
+    }

...which is secret code for old_association = tp_strv_contains (schemes,
uri_scheme);. tp_strv_contains() does the right thing if you pass it a NULL
strv, so the previous block becomes unnecessary.

And then actually I found the pointer arithmetic that followed very hard to
read, so I rewrote the whole function. :) The result is shorter, and (IMHO)
more readable. Top two patches of
<http://git.collabora.co.uk/?p=user/wjt/telepathy-mission-control-wjt.git;a=shortlog;h=refs/heads/account-interface-addressing>.

+    uri_schemes = get_schemes (account_props)
+    assertContains ('scheme-a', uri_schemes)
+    assertContains ('scheme-b', uri_schemes)
+    assertContains ('scheme-c', uri_schemes)
+    assertEquals (len(uri_schemes), 3)

How about assertEquals(set(['scheme-a', 'scheme-b', 'scheme-c']),
set(uri_schemes))? In fact Gabble's copy of servicetest.py has:

def assertSameSets(expected, value):
    exp_set = set(expected)
    val_set = set(value)

    if exp_set != val_set:
        raise AssertionError(
            "expected contents:\n%s\ngot:\n%s" % (
                pretty(exp_set), pretty(val_set)))

to make this easier. Maybe import this to MC's, just after assertEquals() in
servicetest.py (to avoid divergence).

(I don't really like the Association naming, but I can't think of a better one
off-hand.)

A branch seems to exist. Is it ready for review/merge? If so, please set the patch keyword; I'm happy to review it.

Reviewed by wjt, changes made, I think it's ready for merge.

> +	mcd-account-addressing.h \

This should ideally be in libmcd_convenience_la_SOURCES rather than mc_headers, because it doesn't contain any libmissioncontrol-server API.

Other than that, r+. Let's have this in 5.7.0.

Appears to have been fixed in 5.7.0.