Bug 29077 - refactor the roster, moving logic out of the channels into the roster itself
Summary: refactor the roster, moving logic out of the channels into the roster itself
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: haze (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/sm...
Whiteboard: review+ with trivia
Keywords: patch
Depends on: 29076
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-15 08:25 UTC by Simon McVittie
Modified: 2010-09-22 09:53 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2010-07-15 08:25:24 UTC
Haze's contact lists, like Gabble's, have some logic in the channel itself. All of this logic needs to move to the channel manager so it can become a TpBaseContactList.

This branch also adds tests for the contact lists' current behaviour, with good coverage.

http://git.collabora.co.uk/?p=user/smcv/haze-smcv.git;a=shortlog;h=refs/heads/contact-list
Comment 1 Will Thompson 2010-09-22 06:42:36 UTC
Test deleting contact lists via Close()

Is this change merged back to the corresponding test in Gabble?

+    bus.get_object(conn.bus_name, '/').Ping()

sync_dbus?

haze_contact_list_add_to_group() uses:

+    PurpleGroup *group = purple_group_new (group_name);

which is actually kosher, because _new() returns an existing group if possible. It threw me a bit, though. Guess I've been away too long. Any chance of a comment explaining that this is correct?

Otherwise looks fine.
Comment 2 Simon McVittie 2010-09-22 08:16:10 UTC
(In reply to comment #1)
> Test deleting contact lists via Close()
> 
> Is this change merged back to the corresponding test in Gabble?

There's no directly corresponding test, so, no; libpurple's contact list handling differs sufficiently that the tests would have to differ anyway. My TpBaseContactList branch for Gabble does add a test for deleting non-empty groups.

> +    bus.get_object(conn.bus_name, '/').Ping()
> 
> sync_dbus?

Obvious change applied.

> haze_contact_list_add_to_group() uses:
> 
> +    PurpleGroup *group = purple_group_new (group_name);
> 
> which is actually kosher, because _new() returns an existing group if possible.
> It threw me a bit, though. Guess I've been away too long. Any chance of a
> comment explaining that this is correct?

    /* This is correct, despite the naming: it returns a borrowed reference
     * to an existing group (if possible) or to a new group (otherwise). */
    PurpleGroup *group = purple_group_new (group_name);
Comment 3 Will Thompson 2010-09-22 08:59:41 UTC
looks fine, ship it.
Comment 4 Simon McVittie 2010-09-22 09:53:00 UTC
Fixed in git for 0.5.0


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.