Refactor logic out of UploadDiffForm
Review Request #4231 — Created June 11, 2013 and submitted
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.  | 
      
        | 
      |
| 
       
        Col: 20 E221 multiple spaces before operator  | 
      
        | 
      |
| 
       
        It wasn't immediately clear to me how this is "form data," except for where it was called from. I eventually …  | 
      
        | 
      |
| 
       
        Col: 9 E303 too many blank lines (2)  | 
      
        | 
      |
| 
       
        Col: 20 E127 continuation line over-indented for visual indent  | 
      
        | 
      |
| 
       
        Col: 22 E127 continuation line over-indented for visual indent  | 
      
        | 
      |
| 
       
        Could simplify by just passing get_file_exists, and just have that function take self. Same with the others.  | 
      
        | 
      |
| 
       
        Col: 80 E501 line too long (82 > 79 characters)  | 
      
        | 
      |
| 
       
        Col: 38 E127 continuation line over-indented for visual indent  | 
      
        | 
      |
| 
       
        I think they're UploadedFile objects, not FileFields, aren't they?  | 
      
        | 
      
- 
 
 - 
 
 
 - 
 
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.
 - 
 
Could simplify by just passing get_file_exists, and just have that function take self. Same with the others.
 
- 
 
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: 
- 
 
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: 
- 
 
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: 
 
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: