Merge lp://qastaging/~rodrigo-moya/desktopcouch/detailed-api into lp://qastaging/desktopcouch

Proposed by Rodrigo Moya
Status: Merged
Approved by: Eric Casteleijn
Approved revision: 166
Merged at revision: 160
Proposed branch: lp://qastaging/~rodrigo-moya/desktopcouch/detailed-api
Merge into: lp://qastaging/desktopcouch
Diff against target: 218 lines (+150/-13)
5 files modified
desktopcouch/notes/record.py (+42/-6)
desktopcouch/notes/tests/__init__.py (+18/-0)
desktopcouch/notes/tests/test_record.py (+50/-0)
desktopcouch/tasks/record.py (+28/-4)
desktopcouch/tasks/tests/test_record.py (+12/-3)
To merge this branch: bzr merge lp://qastaging/~rodrigo-moya/desktopcouch/detailed-api
Reviewer Review Type Date Requested Status
Eric Casteleijn (community) Approve
Nicola Larosa (community) Approve
Review via email: mp+26026@code.qastaging.launchpad.net

Commit message

Add detailed API for notes and tasks

Description of the change

Add detailed API for notes and tasks

To post a comment you must log in.
160. By Rodrigo Moya

Added missing file

161. By Rodrigo Moya

Add checks for date field values

162. By Rodrigo Moya

Leave the exception handling to the caller

Revision history for this message
Eric Casteleijn (thisfred) wrote :
review: Needs Fixing
163. By Rodrigo Moya

Fixed access to Record's class data and improve tests

164. By Rodrigo Moya

Fix typo

165. By Rodrigo Moya

Fix typo

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

It's better now, thanks. :-) There's still too much copied code for my taste, but I understand it's gonna differentiate later.

Tests pass; I tried running pylint, but it's apparent it's not been run for a long time, if ever, so disregarding it.

The tests use setattr and internal representation, but no getattr, why?

- desktopcouch/notes/tests/test_record.py

  This assertion:

                self.assert_(field_type in ['string', 'date'])

  will always fail, so you may write it in a clearer way:

                self.fail('Unknown field type: %s' % field_type)

review: Approve
166. By Rodrigo Moya

Add getattr() check to tests

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

Looks good now, tests pass

review: Approve

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