Code review comment for lp:~leonardr/launchpadlib/instantiate-resource-from-url

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

« Back to merge proposal