Summary

Pyflakes cleanup

Review Request #81 — Created June 18, 2007 and submitted

Information

Owner

emurphy

Repository

Review Board SVN (deprecated)

Branch

branch branch

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

A bunch of pyflakes cleanup.

I think there are several bugs fixed here (mostly typos).
It would be very cool to come up with a way that pyflakes could be run as part of 'manage.py test', so that the source could be kept clean, but I didn't think of an easy way to do that yet.

Testing Done

test suite still passes.
some of the bugs here would have been found if the code had ever been run.

Most of this looks OK.  Got a few easy changes for you.

emurphy June 19, 2007, 12:54 a.m.

All good changes, will upload a new diff now.

trunk/reviewboard/reviews/json.py (Diff revision 1)

I think the way it was written is correct; this checks the review, not the request being reviewed.

emurphy June 19, 2007, 12:54 a.m.

I botched this change, but the original code still had a problem. Pyflakes was complaining that 'request' was an undefined name. I will change this so the test is against review_request.user, but the JsonResponseError contains the review, not the review request. Does that sound right?

david June 20, 2007, 8:47 a.m.

No, that doesn't sound right.  What needs to happen is that the
request (http request, not review request) needs to get passed
into this function from all of the callers.  You'll notice most
of the functions in this file have a request object.

trunk/reviewboard/reviews/json.py (Diff revision 1)
```
Forbidded?
```
1. EM
  
  emurphy June 19, 2007, 12:54 a.m.
  Doh! looks like I forgot to re-run pyflakes.
trunk/reviewboard/reviews/views.py (Diff revision 1)
```
Please re-wrap these lines
```
1. EM
  
  emurphy June 19, 2007, 12:54 a.m.
  will do.

trunk/reviewboard/reviews/views.py (Diff revision 1)

Another Forbidded.  Did you run pyflakes again after these changes?

emurphy June 19, 2007, 12:55 a.m.

Doh! No, I apparently forgot to do that on this directory.
Will clean up.

trunk/reviewboard/settings.py (Diff revision 1)

emurphy June 19, 2007, 12:56 a.m.

My mistake, I was trying to figure out the diff formats (I've been working using bazaar, which has support for branching from a subversion repository). Will upload a new diff without this in it.

trunk/reviewboard/diffviewer/models.py (Diff revision 1)

Can you add an extra line here?

emurphy June 19, 2007, 1:04 a.m.

sure. now have two blank lines between imports and the start of the code.

trunk/reviewboard/diffviewer/models.py (Diff revision 1)

emurphy June 19, 2007, 1:04 a.m.

more messing about with diffs that should not have been included in this patch.
Sorry for not checking it more carefully before submitting the first time.

trunk/reviewboard/contrib/db/load-db.py (Diff revision 1)
```
Please separate these onto their own lines:
import os
import sys
```
1. EM
  
  emurphy June 19, 2007, 1:07 a.m.
  will do
trunk/reviewboard/scmtools/core.py (Diff revision 1)
```
No reason for these blank lines anymore.
```
1. EM
  
  emurphy June 19, 2007, 1:07 a.m.
  removed.

trunk/reviewboard/scmtools/fooversion.py (Diff revision 1)

We can probably just delete the FooVersion tool, actually.  It was useful for testing early on, but no longer.

chipx86 June 18, 2007, 6:37 p.m.

I actually use it still. It's useful when on the train, since I keep forgetting to get a local SVN repository set up.

emurphy June 19, 2007, 1:08 a.m.

I'll leave FooVersion tool in for the moment, although I'm curious to hear how it is used for testing now that we have a local SVN repo included in the testdata.

Ship it!

OK, looks like you actually had figured out the review vs. request thing in your latest diff.  Committed to svn.