Exclude ancestor FileDiffs from the diffviewer

Review Request #10124 — Created Aug. 22, 2018 and submitted

brennie
Review Board
release-4.0.x
10123
10130
423f1c4...
reviewboard

When a review request is created with commit history, it is likely that
there will be FileDiffs in multiple commits that modify the same file.
For any FileDiff in a commit, all FileDiffs in previous commits are
known as "ancestor" FileDiffs.

These ancestor FileDiffs are now excluded from the diffviewer. This
removes the duplication of seeing the same file listed multiple times
with incremental changes. However, currently only the latest change for
that file is shown. True cumulative diffs are supported in /r/10130/.

  • Ran unit tests.
  • Viewed a review request with commit history and did not see repeated
    files.
  • 0
  • 0
  • 13
  • 1
  • 14
Description From Last Updated
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

brennie
Review request changed

Summary:

-WIP: Exclude ancestor FileDiffs from the diffviewer
+Exclude ancestor FileDiffs from the diffviewer

Testing Done:

~  
  • Viewed a review request with commit history and did not see repeated
    files.
~  
  • TODO: Add unit tests.
  ~
  • Ran unit tests.
  ~
  • Viewed a review request with commit history and did not see repeated
    files.

Commit:

-c099ebcdb374f4577aea2f1cc18afeccc7f24869
+4c34e77856c8096e5bc3043659e898cfa334cd12

Diff:

Revision 2 (+96 -21)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
brennie
chipx86
  1. 
      
  2. The first sentence of the description has some redundancy going on in the first sentence of the description.

    I'm also just having a really hard time understanding this change from the description altogether.

  3. reviewboard/diffviewer/commit_utils.py (Diff revision 5)
     
     

    Maybe this should be called get_leaf_filediffs?

  4. reviewboard/diffviewer/commit_utils.py (Diff revision 5)
     
     

    "FileDiffs"

    I feel like this could be worded better. Maybe:

    python
    """Return only the leafs in the list of FileDiffs."""
    

    With a description detailing what it means to be a leaf.

  5. reviewboard/diffviewer/commit_utils.py (Diff revision 5)
     
     
     

    This should be in the description.

  6. reviewboard/diffviewer/diffutils.py (Diff revision 5)
     
     
     

    Can you move the list() to the next line, so it's more tightly coupled to the queryset?

  7. Alphabetical order.

  8. Missing docstring.

  9. 
      
brennie
chipx86
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (5cc921a)
Loading...