Bug 42348

Summary: Review Chan.I.FileTransfer.Metadata
Product: Telepathy Reporter: Jonny Lamb <jonny.lamb>
Component: salutAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium CC: daniele.domenichelli
Version: git masterKeywords: patch
Hardware: Other   
OS: All   
URL: http://cgit.freedesktop.org/~jonny/telepathy-salut/log/?h=ft-metadata
Whiteboard: review+
i915 platform: i915 features:
Bug Depends on: 39644    
Bug Blocks:    

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.