Pyflakes cleanup

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

Information

Review Board SVN (deprecated)
branch branch

Reviewers

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.
test suite still passes.
some of the bugs here would have been found if the code had ever been run.
david
  1. Most of this looks OK.  Got a few easy changes for you.
    1. All good changes, will upload a new diff now.
  2. 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.
    1. 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?
    2. 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.
  3. trunk/reviewboard/reviews/json.py (Diff revision 1)
     
     
    Forbidded?
    1. Doh! looks like I forgot to re-run pyflakes.
  4. trunk/reviewboard/reviews/views.py (Diff revision 1)
     
     
     
     
    Please re-wrap these lines
  5. trunk/reviewboard/reviews/views.py (Diff revision 1)
     
     
    Another Forbidded.  Did you run pyflakes again after these changes?
    1. Doh! No, I apparently forgot to do that on this directory.
      Will clean up.
  6. trunk/reviewboard/settings.py (Diff revision 1)
     
     
     
     
    ?
    1. 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.
  7. trunk/reviewboard/diffviewer/models.py (Diff revision 1)
     
     
    Can you add an extra line here?
    1. sure. now have two blank lines between imports and the start of the code.
  8. trunk/reviewboard/diffviewer/models.py (Diff revision 1)
     
     
     
     
     
     
     
    ?
    1. 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.
  9. trunk/reviewboard/contrib/db/load-db.py (Diff revision 1)
     
     
    Please separate these onto their own lines:
    import os
    import sys
  10. trunk/reviewboard/scmtools/core.py (Diff revision 1)
     
     
     
    No reason for these blank lines anymore.
  11. We can probably just delete the FooVersion tool, actually.  It was useful for testing early on, but no longer.
    1. I actually use it still. It's useful when on the train, since I keep forgetting to get a local SVN repository set up.
    2. 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.
  12. 
      
david
  1. OK, looks like you actually had figured out the review vs. request thing in your latest diff.  Committed to svn.
  2. 
      
Loading...