- Download Diff

Summary:

Generate diffs between base and tip commits

Review Request #10130 — Created Aug. 27, 2018 and submitted Oct. 18, 2018, 9:39 p.m.

Information
Owner:	brennie
Repository:	Review Board
Branch:	release-4.0.x
Bugs:
Depends On:	~~10124~~
Commit:	45b9779...
Reviewers
Groups:	reviewboard
People:

Description

The get_diff_files utility can now generate a list of files to diff
that exist between two commits in a diffset. This allows us to generate
diffs between two commits (and therefore to see the diff of any
individual commit). There is currently no UI support for this behaviour,
but that is comming in a future patch.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
F811 redefinition of unused 'test_get_diff_files_with_history_tip_commit' from line 1213	reviewbot	Aug. 27, 2018, 2:58 p.m.
Missing space between the backtick and "diffsets".	chipx86	Aug. 30, 2018, 2:08 p.m.
Here, too.	chipx86	Aug. 30, 2018, 2:08 p.m.
Missing blank line.	chipx86	Aug. 30, 2018, 2:08 p.m.
No blank line needed here.	chipx86	Aug. 30, 2018, 2:08 p.m.
Given the repeated complexity of this, it would be nice to make this whole expected_results building into a private utility …	chipx86	Aug. 30, 2018, 2:20 p.m.
Redundant "diffsets" in this sentence.	chipx86	Sept. 4, 2018, 1:03 p.m.
Same here.	chipx86	Sept. 4, 2018, 1:03 p.m.
Can you compare explicitly against 0?	chipx86	Sept. 4, 2018, 1:03 p.m.
Here, too.	chipx86	Sept. 4, 2018, 1:03 p.m.
Should be .. code-block:: python (note the ::).	chipx86	Sept. 4, 2018, 1:04 p.m.
This doesn't match.	chipx86	Sept. 4, 2018, 1:04 p.m.
Syntax is super wonky here (0and). Does this even run?	david	Sept. 27, 2018, 11:02 a.m.
Should be "base commit"	david	Oct. 15, 2018, 1:17 p.m.
This is a little too fancy. How about: for ancestor in reversed(ancestors): if ancestor.commit_id <= base_commit.pk: base_filediff = ancestor break	david	Oct. 15, 2018, 1:18 p.m.
This looks like a fancy way of just doing by_details.get(base_filediff_details).	chipx86	Oct. 16, 2018, 2:28 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 1)

Show all issues

F811 redefinition of unused 'test_get_diff_files_with_history_tip_commit' from line 1213

Change Summary:

Flake8 and typo fixes

Commit:

-	7503455a150e258de558f0dbf2713c05812efc9e
+	b5834139aaa5eeb373c9206ebb2de545836aa248

Diff:

Revision 2 (+332 -12)

Show changes

	reviewboard/diffviewer/commit_utils.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/tests/test_diffutils.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/diffutils.py (Diff revision 2)
Show all issues
```
Missing space between the backtick and "diffsets".
```
reviewboard/diffviewer/diffutils.py (Diff revision 2)
Show all issues
```
Here, too.
```
reviewboard/diffviewer/diffutils.py (Diff revision 2)
Show all issues
```
Missing blank line.
```
reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 2)
Show all issues
```
No blank line needed here.
```

reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 2)

Show all issues

Given the repeated complexity of this, it would be nice to make this whole expected_results building into a private utility method.

Change Summary:

Addressed feedback.

Commit:

-	b5834139aaa5eeb373c9206ebb2de545836aa248
+	9d8e07f93a138d75da763d2474536df69d9a0c22

Diff:

Revision 3 (+354 -14)

Show changes

	reviewboard/diffviewer/commit_utils.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/tests/test_diffutils.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/diffutils.py (Diff revision 3)
Show all issues
```
Redundant "diffsets" in this sentence.
```
reviewboard/diffviewer/diffutils.py (Diff revision 3)
Show all issues
```
Same here.
```
reviewboard/diffviewer/diffutils.py (Diff revision 3)
Show all issues
```
Can you compare explicitly against 0?
```
reviewboard/diffviewer/diffutils.py (Diff revision 3)
Show all issues
```
Here, too.
```
reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 3)
Show all issues
```
Should be .. code-block:: python (note the ::).
```

reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 3)

Show all issues

This doesn't match.

brennie Sept. 4, 2018, 1:04 p.m.

Weird, I must've missed committing the docs change because I have it locally.

Change Summary:

Addressed feedback.

Commit:

-	9d8e07f93a138d75da763d2474536df69d9a0c22
+	5af6c501da5c8b9a449e36bddb2b4af28ece2f78

Diff:

Revision 4 (+352 -12)

Show changes

	reviewboard/diffviewer/commit_utils.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/tests/test_diffutils.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/diffutils.py (Diff revision 4)

Show all issues

Syntax is super wonky here (0and). Does this even run?

brennie Sept. 26, 2018, 1:01 p.m.

Looks like I goofed it up before committing. Oops.

Commit:

-	5af6c501da5c8b9a449e36bddb2b4af28ece2f78
+	55caa2adf3df536281b179f65292df1f21406861

Diff:

Revision 5 (+352 -12)

Show changes

	reviewboard/diffviewer/commit_utils.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/tests/test_diffutils.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/diffutils.py (Diff revision 5)
Show all issues
```
Should be "base commit"
```

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

Show all issues

This is a little too fancy. How about:

for ancestor in reversed(ancestors):
    if ancestor.commit_id <= base_commit.pk:
        base_filediff = ancestor
        break

Change Summary:

Addressed feedback.

Commit:

-	55caa2adf3df536281b179f65292df1f21406861
+	e0d6450582d52547d0402c8818e7dc1983446c8e

Diff:

Revision 6 (+348 -12)

Show changes

	reviewboard/diffviewer/commit_utils.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/tests/test_diffutils.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 6)

Show all issues

This looks like a fancy way of just doing by_details.get(base_filediff_details).

brennie Oct. 16, 2018, 2:28 p.m.
```
Sure is!
```

Change Summary:

Addressed feedback.

Commit:

-	e0d6450582d52547d0402c8818e7dc1983446c8e
+	45b977927e5a9e82f7f24371f48590cd50ae772d

Diff:

Revision 7 (+345 -12)

Show changes

	reviewboard/diffviewer/commit_utils.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/tests/test_diffutils.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (5820e10)

You have a pending review.

Review Board 5.0.2 alpha 0 (dev)

Generate diffs between base and tip commits

flake8

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Status: Closed (submitted)

Change Summary: