flake8
-
reviewboard/diffviewer/diffutils.py (Diff revision 1) Show all issues -
-
reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 1) E501 line too long (83 > 79 characters)
-
Review Request #10122 — Created Aug. 20, 2018 and submitted
If a review request was uploaded with a
FileDiff
that contained a
parent diff that creates a 0 byte file, we would previously be unable to
generate the pre-patch file due to a limitation ofpatch(1)
. To work
around this, when we encounter a specific error from patch (that is,
when it only finds what it considers garbage in the input), we check to
see if the parent diff is empty and if so we return an empty buffer.
Future calls willget_original_file
for that file will have no extra
database query overhead as the results of this (which costs at least two
queries) will be cached with theFileDiff
.
FileDiff
that created a 0 byteDescription | From | Last Updated |
---|---|---|
E127 continuation line over-indented for visual indent |
![]() |
|
E501 line too long (90 > 79 characters) |
![]() |
|
E501 line too long (83 > 79 characters) |
![]() |
|
W391 blank line at end of file |
![]() |
|
Can you move the parens to be around the string instead? Same effect, but keeps the string nicely grouped together. |
|
|
I think this would all be better as a method on FileDiff. |
|
|
No parens around these equality checks. |
|
|
Blank line between these. |
|
|
Missing a test docstring. |
|
|
Can you pass these as keyword arguments? |
|
|
Can you add a comment saying why we're doing this again? |
|
|
Here, too. |
|
|
W503 line break before binary operator |
![]() |
|
Did you mean for the next clause to be outside of these parens, or should the parens go away? |
|
|
Given the previous issue, I think we need better test coverage for the various cases here. |
|
|
F821 undefined name 'parent_fiel' |
![]() |
reviewboard/diffviewer/diffutils.py (Diff revision 1) |
---|
reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 1) |
---|
E501 line too long (83 > 79 characters)
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+76 -3) |
reviewboard/diffviewer/diffutils.py (Diff revision 2) |
---|
Can you move the parens to be around the string instead? Same effect, but keeps the string nicely grouped together.
reviewboard/diffviewer/diffutils.py (Diff revision 2) |
---|
I think this would all be better as a method on
FileDiff
.
reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 2) |
---|
Can you pass these as keyword arguments?
reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 2) |
---|
Can you add a comment saying why we're doing this again?
Addressed issues.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+116 -3) |
Flake8
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+116 -3) |
reviewboard/diffviewer/diffutils.py (Diff revision 4) |
---|
Did you mean for the next clause to be outside of these parens, or should the parens go away?
Addressed Christian's feedback.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+116 -3) |
reviewboard/diffviewer/diffutils.py (Diff revision 5) |
---|
Given the previous issue, I think we need better test coverage for the various cases here.
Addressed Christian's feedback.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+123 -7) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 7 (+123 -7) |