Refactor logic out of UploadDiffForm

Review Request #4231 — Created June 11, 2013 and submitted

Information

Review Board
master

Reviewers

Refactor logic out of UploadDiffForm

All of the logic for taking a diff file and creating a DiffSet was in
UploadDiffForm. This change moves it out into a manager, so that I'll be able to
use the same code for creating DiffSets from diff data fetched from an upstream
repository.
Ran unit tests.
Description From Last Updated

Same line.

chipx86chipx86

Col: 20 E221 multiple spaces before operator

reviewbotreviewbot

It wasn't immediately clear to me how this is "form data," except for where it was called from. I eventually …

chipx86chipx86

Col: 9 E303 too many blank lines (2)

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

Could simplify by just passing get_file_exists, and just have that function take self. Same with the others.

chipx86chipx86

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

reviewbotreviewbot

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

reviewbotreviewbot

I think they're UploadedFile objects, not FileFields, aren't they?

chipx86chipx86
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/diffviewer/forms.py
        reviewboard/diffviewer/models.py
        reviewboard/diffviewer/managers.py
        reviewboard/diffviewer/tests.py
      Ignored Files:
    
    
  2. reviewboard/diffviewer/managers.py (Diff revision 1)
     
     
    Col: 20
     E221 multiple spaces before operator
    
  3. reviewboard/diffviewer/managers.py (Diff revision 1)
     
     
    Col: 9
     E303 too many blank lines (2)
    
  4. reviewboard/diffviewer/managers.py (Diff revision 1)
     
     
    Col: 20
     E127 continuation line over-indented for visual indent
    
  5. reviewboard/diffviewer/managers.py (Diff revision 1)
     
     
    Col: 22
     E127 continuation line over-indented for visual indent
    
  6. reviewboard/diffviewer/tests.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  7. reviewboard/diffviewer/tests.py (Diff revision 1)
     
     
    Col: 38
     E127 continuation line over-indented for visual indent
    
  8. 
      
chipx86
  1. 
      
  2. reviewboard/diffviewer/managers.py (Diff revision 1)
     
     
     
    Same line.
  3. reviewboard/diffviewer/managers.py (Diff revision 1)
     
     
    It wasn't immediately clear to me how this is "form data," except for where it was called from. I eventually realized the diff_file and parent_diff_file are UploadedFile objects from a form. Maybe rename this to create_from_upload?
    
    Also, can you add docstrings for this and create_from_data? It'll help make it clear how they differ.
    1. What were your thoughts on the name change?
    2. OK.
  4. reviewboard/diffviewer/tests.py (Diff revision 1)
     
     
     
    Could simplify by just passing get_file_exists, and just have that function take self. Same with the others.
  5. 
      
david
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/diffviewer/forms.py
        reviewboard/diffviewer/models.py
        reviewboard/diffviewer/managers.py
        reviewboard/diffviewer/tests.py
      Ignored Files:
    
    
  2. 
      
chipx86
  1. 
      
  2. reviewboard/diffviewer/managers.py (Diff revision 2)
     
     
    I think they're UploadedFile objects, not FileFields, aren't they?
  3. 
      
david
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/diffviewer/forms.py
        reviewboard/diffviewer/models.py
        reviewboard/diffviewer/managers.py
        reviewboard/diffviewer/tests.py
      Ignored Files:
    
    
  2. 
      
chipx86
  1. Ship It!
  2. 
      
david
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/webapi/resources.py
        reviewboard/diffviewer/models.py
        reviewboard/diffviewer/errors.py
        reviewboard/diffviewer/tests.py
        reviewboard/reviews/forms.py
        reviewboard/diffviewer/forms.py
        reviewboard/diffviewer/managers.py
      Ignored Files:
    
    
  2. 
      
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (2d8174d).
Loading...