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