Code review comment for lp:~benji/lazr.restful/bug-413174

Revision history for this message
Benji York (benji) wrote :

On Mon, Sep 6, 2010 at 1:48 PM, Gary Poster <email address hidden> wrote:
> Review: Approve
> Very nice catch with the check for isinstance of BaseException.
>
> I question the view pattern for the registration and lookup--using
> "index.html" as the distinguishing marker.  I'd prefer a custom interface
> myself.  If, in retrospect, you agree, great.  If not, I can see where you
> are coming from: leave as is.

I was following the pattern used for the other error views. I don't feel
strongly about it one way or the other so I'll change it.

> We need a linter.

Yep. I'll take a quick look to see if pyflakes or similar is easily
usable with our lazr buildouts.

> Either ``from types import ClassType`` in
> src/lazr/restful/error.py shouldn't be there now, or it should have been
> caught earlier.  Which is it?

That's left over from a previous revision of the code. Removed.

> In error.py, you say "# Bug 11512 describes the possibility of controlling
> the HTTP status of...."  I think you mean bug 631711.

Indeed. Fixed.
--
Benji York

« Back to merge proposal