launchpadlib

Merge lp:~leonardr/launchpadlib/instantiate-resource-from-url into lp:~launchpad-pqm/launchpadlib/devel

  1. instantiate-resource-from-url
  2. Merge into devel
Proposed by Leonard Richardson
Status: Merged
Merged at revision: not available
Proposed branch: lp:~leonardr/launchpadlib/instantiate-resource-from-url
Merge into: lp:~launchpad-pqm/launchpadlib/devel
To merge this branch: bzr merge lp:~leonardr/launchpadlib/instantiate-resource-from-url
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+728@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

It all looks good, except that I can't run the tests even after linking it into a current LP tree. I first get a "No module named Mailman.MailList", but I can't believe that's your problem.

> + return Resource._wrap_resource(

You could do `self._wrap_resource` here. That might be more appropriate, especially for anyone who wants to subclass this.

> + self._root, wadl_resource, representation, 'application/json',

'application/json' is the default for `representation_media_type`, but it does no harm I guess.

review: Approve
Revision history for this message
Leonard Richardson (leonardr) wrote :

I changed Resource._wrap_resource to self._wrap_resource in several places in different subclasses of Resource. Good catch. I'd like to leave the application/json in place for now.

lp:~leonardr/launchpadlib/instantiate-resource-from-url updated
12. By Leonard Richardson

Response to feedback.

Subscribers

People subscribed via source and target branches