Better Moved Files
Review Request #2905 — Created Feb. 19, 2012 and submitted
Uses info from the diff to detect if files have been moved and displays the result in the diff viewer. This adds the functionality of parsing git's extended header of detecting moved files. Before, the diff would show two changes: a deleted file and a new file. Now, there is only one change and it is displayed correctly in the diff viewer. Since a file may be moved and have no actual diff data (and no revision), some extra checks are now required to compensate for this. These may be different depending on the SCM, so a more generic approach might be required.
Using "git diff -M" to manually create a patch that contains: -Files that have been moved and not edited -Files that have been both moved and edited (post-review has been updated but has not been posted yet, so to test you must manually create a diff)
Screenshots
| Description | From | Last Updated |
|---|---|---|
|
For multi-line conditions, use parens instead of \. Also make sure that the alignment is consistent on each. |
 chipx86
|
|
|
Blank line before this. |
chipx86
|
|
|
I know we didn't do this with deleted, but this should compare against self.MOVED. (You can update deleted to do … |
chipx86
|
|
|
This should go inside the conditional's body. So: if .... # Moved ... |
chipx86
|
|
|
I don't love the negation method. I find it's easier to read it if each check stands alone whenever possible. |
chipx86
|
|
|
Best not to modify unrelated lines. It distracts from the main purpose of the review, muddies up the history of … |
chipx86
|
|
|
Using Django 1.4 (that will be required as soon as an upstream thing is fixed in a module we depend … |
chipx86
|
|
|
"length" |
chipx86
|
|
|
If you add a "(moved)" suffix to the file name, this can just be: "No changes were made to this … |
chipx86
|
DD
- Change Summary:
-
Added a screenshot of a moved and edited file.
- Screenshots:
-
A moved file with changes
-
-
For multi-line conditions, use parens instead of \. Also make sure that the alignment is consistent on each.
-
-
I know we didn't do this with deleted, but this should compare against self.MOVED. (You can update deleted to do that too.)
-
-
I don't love the negation method. I find it's easier to read it if each check stands alone whenever possible.
-
Best not to modify unrelated lines. It distracts from the main purpose of the review, muddies up the history of lines a bit, and can cause merge conflicts if other people modify the same lines. In this case, that's exactly what's happening, because I rewrote these functions, so there will be some merge ugliness.
-
Using Django 1.4 (that will be required as soon as an upstream thing is fixed in a module we depend on), you can change this to use {% elif ... %} -
-
If you add a "(moved)" suffix to the file name, this can just be: "No changes were made to this file."
DD
- Change Summary:
-
Made changes that were brought up in reviews.
- Diff:
-
Revision 2 (+80 -47)
- Screenshots:
-
A moved file with no dataA moved file with changesMoved and not changed file.Moved and changed file.