[Bug 30461] New: Review Conn.I.Resources

[bugzilla-daemon at freedesktop.org]

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

           Summary: Review Conn.I.Resources
           Product: Telepathy
           Version: git master
          Platform: Other
               URL: http://git.collabora.co.uk/?p=user/jonny/telepathy-gab
                    ble.git;a=shortlog;h=refs/heads/resources
        OS/Version: All
            Status: NEW
          Keywords: patch
          Severity: normal
          Priority: medium
         Component: gabble
        AssignedTo: telepathy-bugs at lists.freedesktop.org
        ReportedBy: jonny.lamb at collabora.co.uk
         QAContact: telepathy-bugs at lists.freedesktop.org


Simon already made a few comments:

Regarding Gabble:

Regression tests are good, and should have caught this:

> +  if (name == g_quark_from_static_string ("MailNotificationFlags"))
> +    g_value_set_uint (value, GABBLE_RESOURCES_HUMAN_READABILITY_MAYBE);
                                              ^^^^^^^^^^^^^^^^^^^^^

> +static const gchar *
> +presence_id_to_status (GabblePresenceId id,
> +    TpConnectionPresenceType *type)

Surely this *must* be duplicating something in conn-presence.c? (It's entirely
possible that the information exists in conn-presence.c but needs some
refactoring, though.)

(Be particularly careful about Senko's new plugin-defined presences, which can
be visible on the self-handle (if the self-resource is exposed) and didn't
exist when you wrote this branch...)

> +      val = tp_g_value_slice_new_boxed (
> +          GABBLE_HASH_TYPE_RESOURCE_INFORMATION_MAP, resources);
[...]
> +      g_hash_table_unref (resources);

Just take_boxed to avoid a copy?

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