Bug 42348 - Review Chan.I.FileTransfer.Metadata
Summary: Review Chan.I.FileTransfer.Metadata
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: salut (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.freedesktop.org/~jonny/te...
Whiteboard: review+
Keywords: patch
Depends on: 39644
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-28 10:48 UTC by Jonny Lamb
Modified: 2011-11-16 00:59 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Jonny Lamb 2011-10-28 10:48:55 UTC
This is just like bug #42288 but for Salut.

The proto-XEP doesn't really apply here because it doesn't use SI file transfers, it uses XEP-0066. Oh well.

The implementation should be pretty much line-for-line the same as Gabble, so if you review one of them then the other is basically exactly the same.

Go go power rangers!
Comment 1 Will Thompson 2011-11-09 09:05:48 UTC
In add_metadata_forms:

+      else
+        {
+          list = g_list_append (list, form);
+        }

Is this in the Gabble branch too? Use a GQueue. I guess I got distracted by the yucky tree building.

The rest looks very familiar. I guess some of my review comments from the Gabble bug may also apply here?
Comment 2 Jonny Lamb 2011-11-09 09:35:43 UTC
(In reply to comment #1)
> In add_metadata_forms:
> 
> +      else
> +        {
> +          list = g_list_append (list, form);
> +        }
> 
> Is this in the Gabble branch too? Use a GQueue. I guess I got distracted by the
> yucky tree building.

Fixed! It's not in Gabble.

> The rest looks very familiar. I guess some of my review comments from the
> Gabble bug may also apply here?

Yes, I have cherry-picked the appropriate ones, which are most of them.
Comment 3 Will Thompson 2011-11-15 07:02:09 UTC
Looks good—though if i were to nitpick i'd point out that the FT.I.Metadata tests don't try specifying more than one value for a key.
Comment 4 Jonny Lamb 2011-11-16 00:59:20 UTC
Merged, thanks.


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.