GNOME Bugzilla – Bug 655609
Implement setter for URLs
Last modified: 2011-08-13 16:59:52 UTC
22:10:09 pwithnall | rgs_: The urls property isn't settable 22:10:51 rgs_ | pwithnall: thats actually me being a slacker. 22:11:56 pwithnall | :) 22:12:52 * | rgs_ checks how urls are managed in e-contact.h 22:14:22 rgs_ | pwithnall: so we've got: http://www.fpaste.org/ezeq/ 22:14:45 rgs_ | pwithnall: I am guessing we care only about the 1st two.. for the time being. Though I am worried there isn't a single multi-purpose list of urls.. 22:15:05 pwithnall | rgs_: Add a custom X-URIS vCard attribute for the rest, I guess 22:15:07 rgs_ | pwithnall: which raises the question: what do we do if we have more urls than space to store 22:15:12 rgs_ | pwithnall: makes sense 22:15:13 * | pwithnall checks what Google Contacts does 22:16:43 pwithnall | rgs_: eds' Google Contacts backend just uses HOMEPAGE_URL and BLOG_URL
First take on this: http://cgit.collabora.com/git/user/rgs/folks/log/?h=bgo-655609 I'll follow up with a test case.
Created attachment 193075 [details] [review] Add support to set urls on Edsf.Persona A couple of corrections here and there.
Created attachment 193076 [details] [review] Add a test for setting URLs And a small test case.
I've added a couple of helps to ease up checking if 2 sets of Urls are the same, so the attached patches are deprecated in favour of this branch: http://cgit.collabora.com/git/user/rgs/folks/log/?h=bgo-655609
Could you post a squashed patch of the branch please? From a quick look, the changes you've made to FieldDetails look very likely to collide head-on with Travis' port to AbstractFieldDetails (bug #653680 and friends). Another thought which comes to mind is that Edsf.Persona.url_properties is quite a good candidate for being moved into UrlDetails. The entries in it are generic enough to label URLs coming from many different backends.
Created attachment 193112 [details] [review] Squashed series of patches to add support for URLs I'll actually wait for Travis' changes to land, but I guess the test and the core changes in Edsf.PersonaStore and Edsf.Persona can get some review in the meanwhile.
Created attachment 193134 [details] [review] Squashed series of patches to add support for URLs A little refresh while I wait for #653680 to land.
Review of attachment 193134 [details] [review]: Here are a few things I spotted on a quick review. My points from comment #5 are still relevant, I think. ::: backends/eds/lib/edsf-persona-store.vala @@ +215,3 @@ * - PersonaDetail.detail_key (PersonaDetail.WEB_SERVICE_ADDRESSES) * - PersonaStore.detail_key (PersonaDetail.NOTES) + * - PersonaStore.detail_key (PersonaDetail.URL) s/URL/URLS/ ::: folks/field-details.vala @@ +162,3 @@ + * @param other the FieldDetail to compare with + * + * @since UNRELEASED Missing a @return line. ::: tests/eds/set-urls.vala @@ +74,3 @@ + + v = Value (typeof (string)); + v.set_string ("Albus Percival Wulfric Brian Dumbledore"); I approve.
commit f4f596bddedcc64670be03ec2ffa46e0d142a720 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Sat Aug 13 12:33:39 2011 +0200 Write support for URLs in Edsf.Persona Closes: bgo#655609 - Implement setter for URLs