1217: Enhanced error detection when uploading diff
- Duplicate
- Review Board
| madc****@gmai***** (Google Code) (Is this you? Claim this profile.) | |
| Sept. 21, 2009 | |
What version are you running? ReviewBoard-1.0alpha4 What's the URL of the page this enhancement relates to, if any? Selecting "Update Diff" on an existing review Describe the enhancement and the motivation for it. Occasionally, if there's a problem with a subsequent diff when trying to update an existing review, the diff file will be accepted and stored with no error checking, then trying to view that diff will result in a python exception. You can then update the review again with a new, correct diff, and viewing that revision of the review will work correctly, however that "bad" revision is permanently in the review, and will break every time you try to view it. If the "Update Diff" functionality had a preview, the uploader could choose to either publish or discard that updated diff. To that end, reviewboard could attempt the shell `patch` call, and upon exception, automatically display an error for the new diff instead of accepting the diff as a permanent part of the review. I'll try to work up an example that shows this behavior and respond with an attachment. What operating system are you using? What browser? Mac OS X 10.5.7; Firefox 3.0.11 Please provide any additional information below.
Here's one example that's not exactly what is described above, but is kind of
related. It's due to subsequent diffs adding files that didn't exist in the original
diff. E.g.:
$ vi foo.txt # new file
$ svn add foo.txt
$ svn diff > rb-1.diff
$ vi foo.txt # edit
$ vi bar.txt # new file
$ svn add bar.txt
$ svn diff > rb-2.diff
Because bar.txt didn't exist in the original diff, adding it in rb-2.diff results in
an exception when viewing the differences between r1 and r2:
Traceback (most recent call last):
File
"/usr/local/lib/python2.6/site-packages/ReviewBoard-1.0alpha4-py2.6.egg/reviewboard/diffviewer/views.py",
line 145, in view_diff
interdiffset, highlighting, True)[0]
IndexError: list index out of range
The desired behavior would be that is shows the modification to foo.txt, and the
addition of bar.txt.
-
+ +
I can't seem to trigger the error as mentioned in the original report. However, I do see that Update Diff currently does have a preview that requires publish or discard, which apparently I didn't realize before. That pretty much makes the original report invalid, though the error in c1 still occurs.
