Changed from hardcoded http codes to using constants from httplib

Review Request #7625 — Created Sept. 13, 2015 and submitted

Information

RBTools
master

Reviewers

Changed from hardcoded 304 and 401 http codes to using the NOT_MODIFIED and UNAUTHORIZED constants from httplib for better readability.

RBTools nose test results - all passed fine


Ran 166 tests in 3.966s

OK (SKIP=50)

Description From Last Updated

Col: 80 E501 line too long (113 > 79 characters)

reviewbotreviewbot

Col: 25 E127 continuation line over-indented for visual indent

reviewbotreviewbot

I think we want to keep 200 and 300 as numbers, and only use the symbolic error for NOT_MODIFIED.

daviddavid

Don't add this line (io and json are still standard-library modules).

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/api/request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/api/request.py
    
    
  2. rbtools/api/request.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (113 > 79 characters)
    
  3. 
      
AH
AH
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        rbtools/api/request.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        rbtools/api/request.py
    
    
  2. rbtools/api/request.py (Diff revision 2)
     
     
    Show all issues
    Col: 25
     E127 continuation line over-indented for visual indent
    
  3. 
      
AH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/api/request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/api/request.py
    
    
  2. 
      
AH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/api/transport/sync.py
        rbtools/api/request.py
        rbtools/commands/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/api/transport/sync.py
        rbtools/api/request.py
        rbtools/commands/__init__.py
    
    
  2. 
      
AH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/api/request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/api/request.py
    
    
  2. 
      
david
  1. 
      
  2. rbtools/api/request.py (Diff revision 5)
     
     
    Show all issues

    I think we want to keep 200 and 300 as numbers, and only use the symbolic error for NOT_MODIFIED.

  3. 
      
AH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/api/request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/api/request.py
    
    
  2. 
      
david
  1. Looking pretty close now!

  2. rbtools/api/request.py (Diff revision 6)
     
     
    Show all issues

    Don't add this line (io and json are still standard-library modules).

    1. Oh interesting. I left the space there because when looking a at the diff view between revision 0 and 1, it shows a space on the original. But looking at the master branch I can see that there was no space.
      Gonna remove that!

  3. 
      
AH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/api/request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/api/request.py
    
    
  2. 
      
AH
brennie
  1. <p>We don't need the issue number in the summary. Other than that it looks fine.</p>

  2. 
      
AH
chipx86
  1. Make sure to follow our guide on choosing a good summary and description: https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/

    Your summary should never mention bug numbers (those will be provided automatically in the description when the change lands), and your descriptions should explain the why and how.

  2. 
      
AH
AH
chipx86
  1. Looks good.

    Also, regarding the description, it's always good to run tests for every change, no matter what :)

    1. Running all the tests is usually best practice.

  2. 
      
mike_conley
  1. Hey Anthony, would you mind running the tests with this patch to ensure all is well, and then posting the results in Testing Done?

  2. 
      
AH
brennie
  1. Ship It!
  2. 
      
AH
Review request changed
Status:
Completed
Change Summary:
Pushed to release-0.7.x (72ccc5d)