GNOME Bugzilla – Bug 645989
Ensure add_persona_from_details handles the basic attributes
Last modified: 2011-04-07 09:36:51 UTC
Created attachment 184489 [details] [review] Add helpers for add_persona_from_details Right now when using add_persona_from_details () there is no definition of the basic keys that the details HashTable param accepts. The attached patch adds the detail_key () method along with the PersonaDetail enum to allow the caller to create the details HashTable without resorting to magic strings and it can also be used by PersonaStores implementing add_persona_from_details to check if the received values are within the basic supported attributes.
Review of attachment 184489 [details] [review]: Seems useful, though the documentation needs a little work. It might also be helpful to amend the documentation for Tpf.PersonaStore.add_persona_from_details() and Trf.PersonaStore.add_persona_from_details() to mention which of the enum values they accept. ::: NEWS @@ +17,3 @@ +* Add detail_key () along with an enum PersonaDetail to PersonaStore + which together define the basic attributes that should be supported + by add_persona_from_details (). Don't forget to list this bug in the “Bugs fixed” section. :-) ::: folks/persona-store.vala @@ +101,3 @@ } +public enum Folks.PersonaDetail This needs a documentation comment, including a @since taglet. @@ +132,3 @@ /** + * The following list of properties are the basic keys + * that each PersonaStore with write capability should s/capability/capabilities/ @@ +133,3 @@ + * The following list of properties are the basic keys + * that each PersonaStore with write capability should + * support for the add_persona_from_details. s/for the/for/ @@ +135,3 @@ + * support for the add_persona_from_details. + * + * Note that this aren't the only valid keys; backends are s/this/these/ @@ +139,3 @@ + * which might be specific to the backend in question. + * + * Should be kept in sync with Folks.PersonaDetail. Even though it's not public, it can't hurt to put {@link}s and @since taglets in this documentation comment. @@ +149,3 @@ + "birthday", + "gender", + "emails", Should be “email-addresses”? @@ +159,3 @@ + + /** + * Returns a key to be used in the details param of add_persona_from_details. add_persona_from_details should be {@link}ed. @@ +161,3 @@ + * Returns a key to be used in the details param of add_persona_from_details. + * + * @param detail the {@link PersonaDetail} to remove I'm not sure what this is supposed to mean. @@ +371,3 @@ * If the details are not recognised or are invalid, + * {@link PersonaStoreError.INVALID_ARGUMENT} will be thrown. A default set + * of possible details are defined at by Folks.PersonaDetail but backends s/defined at by/defined by/ Folks.PersonaDetail should be {@link}ed.
Created attachment 184715 [details] [review] Add helpers for add_persona_from_details Updated according to received reviews. Some of the comments have been addressed in separate patches.
Created attachment 184716 [details] [review] Update Kf.PersonaStore.add_persona_from_details' comments and use defined constants
Since Trf.PersonaStore.add_persona_from_details doesn't use any pre-defined key I prefer to leave its comments as is.
Review of attachment 184715 [details] [review]: Looks good to me, just fix this comment and apply: + * Returns a key to be used in the details param of + * {@link Persona.add_persona_from_details}. Should be something like "Returns the key corresponding to @detail, for use in the details param of {@link Persona.add_persona_from_details}."
Review of attachment 184716 [details] [review]: Looks good.
Review of attachment 184715 [details] [review]: ::: folks/persona-store.vala @@ +116,3 @@ + BIRTHDAY, + GENDER, + EMAILS, I missed this in my first review, but this should be EMAIL_ADDRESSES. Please change it before you commit. :-)
Merged: commit 8435b42f7a0cb1cb09cdf42ce172420dc9f0a60e Author: Raul Gutierrez Segales <raul.gutierrez.segales@collabora.co.uk> Date: Thu Mar 31 15:37:21 2011 +0100 Access detail_key via `this` since its an instance method backends/key-file/kf-persona-store.vala | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) commit fbf1d5eebb24a53b615c1e0a564c57f8885c5e67 Author: Raul Gutierrez Segales <raul.gutierrez.segales@collabora.co.uk> Date: Wed Mar 30 17:49:10 2011 +0100 Update Kf.PersonaStore.add_persona_from_details to use defined constants backends/key-file/kf-persona-store.vala | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) commit e198a0cd1e9717e711c32f8a4503c66a0768e82e Author: Raul Gutierrez Segales <raul.gutierrez.segales@collabora.co.uk> Date: Mon Mar 28 18:52:08 2011 +0100 Helpers to check add_persona_from_details' compliance with basic attributes. The detail_key () method (along with the PersonaDetail enum) allows us to easily check the default attributes that should be handled in the HashTable received by add_persona_from_details. NEWS | 4 ++ folks/persona-store.vala | 73 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) commit 5dd9f0cca03880739e9a3b37fe816070c0b5e5cf Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Sat Apr 2 12:51:27 2011 -0700 Make Folks.PersonaStore.detail_key() static. backends/key-file/kf-persona-store.vala | 4 +- backends/tracker/lib/trf-persona-store.vala | 35 +++++++++++++++---------- folks/persona-store.vala | 4 +- tests/tracker/add-persona.vala | 37 ++++++++++++++++----------- tests/tracker/remove-persona.vala | 5 ++- 5 files changed, 50 insertions(+), 35 deletions(-)