Merge lp://qastaging/~teknico/desktopcouch/contacts-detailed-api into lp://qastaging/desktopcouch

Proposed by Nicola Larosa
Status: Merged
Approved by: Eric Casteleijn
Approved revision: 161
Merged at revision: 161
Proposed branch: lp://qastaging/~teknico/desktopcouch/contacts-detailed-api
Merge into: lp://qastaging/desktopcouch
Diff against target: 102 lines (+59/-11)
2 files modified
desktopcouch/contacts/record.py (+41/-8)
desktopcouch/contacts/tests/test_record.py (+18/-3)
To merge this branch: bzr merge lp://qastaging/~teknico/desktopcouch/contacts-detailed-api
Reviewer Review Type Date Requested Status
Eric Casteleijn (community) Approve
Rodrigo Moya (community) Approve
Review via email: mp+26021@code.qastaging.launchpad.net

Commit message

First cut of a dynamically generated Contacts API, allowing clients to avoid direct access to record fields.

Description of the change

First cut of a dynamically generated Contacts API, allowing clients to avoid direct access to record fields.

To post a comment you must log in.
Revision history for this message
Rodrigo Moya (rodrigo-moya) wrote :

Looks great and all tests pass

review: Approve
Revision history for this message
Eric Casteleijn (thisfred) wrote :

Should we think about having this do a .get instead, or is it ok to get a KeyError when the field is not there?

def fget(self):
    return self.__dict__[field_name]

Revision history for this message
Eric Casteleijn (thisfred) wrote :

Also, I do not quite understand the implications of setting and getting the values in the __dict__. Doesn't that mean they don't end up in the record's data?

If I do this:

foo = Contact()

foo.nick_name = 'Nick'
print foo.nick_name
print foo.keys()

I get:

Nick
['record_type']

and when I do the same on trunk, I get *exactly* the same results. Since the getter and setter don't do anything a normal python object doesn't already allow.

review: Needs Fixing
161. By Nicola Larosa

Fix setters, getters, and test.

Revision history for this message
Nicola Larosa (teknico) wrote :

Eric, you are right, I was setting/getting attributes, rather than record items. I fixed the getter and setter, and strengthened the test, as per our discussion.

Revision history for this message
Eric Casteleijn (thisfred) wrote :

Looks good now, thanks for improving the tests! (all green)

review: Approve
162. By Nicola Larosa

Merge from desktopcouch

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches