Summary

Show commit history differences in interdiffs

Review Request #7219 — Created April 17, 2015 and submitted April 23, 2015, 3:17 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

dvcs

Bugs

Depends On

~~7220~~

Commit

1b90346...

Reviewers

Groups

reviewboard

People

Description

The diffviewer now shows the differences between commit histories in a
manner similar to how the change descriptions show differences in the
commit histories between revisions.

Testing Done

Ran unit tests.

Files

Issues

Description	From	Last Updated
Each case should be its own unit test, to help when things change down the road, and to keep this …	chipx86	April 20, 2015, 9:52 p.m.
Blank line between these.	chipx86	April 20, 2015, 9:53 p.m.
Let's raise a ValueError here with a suitable error, to help diagnose issues that may come up down the road.	chipx86	April 20, 2015, 9:54 p.m.
While a minor performance improvement, we should probably pull out history_entry[key] into a local variable so we don't have to …	chipx86	April 20, 2015, 9:54 p.m.
This should be imported a couple lines later.	chipx86	April 20, 2015, 10:05 p.m.
This should also raise a ValueError.	chipx86	April 20, 2015, 9:56 p.m.
We should pull out commit_dict[key] here as well.	chipx86	April 20, 2015, 9:56 p.m.
The list comprehension will be faster, since it doesn't involve a lambda call or a conversion back into a list. …	chipx86	April 20, 2015, 9:59 p.m.
Same here.	chipx86	April 20, 2015, 9:59 p.m.
There should be a blank line before each comment.	chipx86	April 20, 2015, 10 p.m.
This appears to conflict with the value. We should maybe say that if there is no interdiff, the API will …	chipx86	April 20, 2015, 10 p.m.
Space after switch.	chipx86	April 20, 2015, 10:01 p.m.
Blank line between these.	chipx86	April 20, 2015, 10:01 p.m.
Blank line between these.	chipx86	April 20, 2015, 10:01 p.m.
Col: 5 E301 expected 1 blank line, found 0	reviewbot	April 21, 2015, 6:23 a.m.
Should be indented only 4 spaces, not 30	david	April 23, 2015, 3:15 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/builtin_fields.py
    reviewboard/diffviewer/views.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/commitutils.py

Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
    reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/builtin_fields.py
    reviewboard/diffviewer/views.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/commitutils.py

Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
    reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js

Added a commit. (Revision 1)

In change descriptions, we highlight a changed commit with green (in the case of added) or red (in the case of removed).

I haven't added this to the diffviewer commits diffs because I think it will be harder to read with the diff icons AND the row backgrounds.

Thoughts?

david April 22, 2015, 12:49 p.m.

What if you surrounded the diff icon with a circle of white?

brennie April 22, 2015, 8:31 p.m.

The current screenshots reflect this change.

Depends On:: ~~7220 - Refactor CSS for DiffCommit listings~~