Summary

Add a manager for the DiffCommit model

Review Request #9631 — Created Feb. 12, 2018 and submitted July 31, 2018, 2:11 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

~~10079~~, ~~9609~~

Blocks

9642

Commit

bc4b0ce...

Reviewers

Groups

reviewboard

People

Description

The main logic of the DiffSetManager has been refactored into two
parts:

reviewboard.diffviewer.filediff_creator, which handles the
actual processing of diff files into FileDiffs; and
DiffManagerBase, which provides a base implementation of
create_from_upload for both the DiffSetManager and the
DiffCommitManager.

The new DiffCommitManager is able to create DiffCommit objects from
either uploaded files or from data, just as the DiffSetManager can for
DiffSet objects.

This patch also adds DiffSetManager.create_empty to create empty
DiffSets which will contain DiffCommits.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
F401 'nose' imported but unused	reviewbot	Feb. 12, 2018, 8:26 p.m.
typo: reivewboard	david	Feb. 21, 2018, 2:10 p.m.
Missing a period.	david	Feb. 21, 2018, 2:10 p.m.
) should be on the line before.	david	Feb. 21, 2018, 2:10 p.m.
Docstring?	david	Feb. 21, 2018, 2:11 p.m.
There are so many args here, let's put one per line.	david	Feb. 21, 2018, 2:11 p.m.
Can we put one arg per line?	david	Feb. 21, 2018, 2:12 p.m.
) should be on the previous line and no trailing comma after revision=1	david	Feb. 21, 2018, 2:12 p.m.
F841 local variable 'history' is assigned to but never used	reviewbot	Feb. 15, 2018, 3:26 p.m.
W391 blank line at end of file	reviewbot	Feb. 15, 2018, 3:26 p.m.
While here, can you make request default to None and be optional? It's used for an eventual call to get_file_exists(), …	chipx86	Feb. 27, 2018, 7:43 p.m.
"will be empty"	chipx86	Feb. 27, 2018, 7:43 p.m.
Can you pass these as keyword arguments? It'll be easier for maintenance.	chipx86	Feb. 27, 2018, 7:45 p.m.
"filenames"	chipx86	Feb. 27, 2018, 7:45 p.m.
"filename"	chipx86	Feb. 27, 2018, 7:45 p.m.
"filename"	chipx86	Feb. 27, 2018, 7:45 p.m.
"filenames"	chipx86	Feb. 27, 2018, 7:45 p.m.
Can you fix the indentation while you're here?	chipx86	Feb. 27, 2018, 7:45 p.m.
Missing a docstring.	chipx86	Feb. 27, 2018, 7:47 p.m.
Missing a docstring.	chipx86	Feb. 27, 2018, 7:47 p.m.
This read better the way it was before.	chipx86	Feb. 27, 2018, 7:49 p.m.
It's not valid to put more text here between sections.	chipx86	Feb. 27, 2018, 7:49 p.m.
Can you revert this? It doesn't simplify the code any, and makes it less future-proof. Better to have one task …	chipx86	Feb. 27, 2018, 7:49 p.m.
Like above, can we make this optional?	chipx86	Feb. 27, 2018, 7:50 p.m.
Is this necessary, given that this is a new manager for a new model? If it is, we should document …	chipx86	Feb. 27, 2018, 7:50 p.m.
Can you pass a keyword argument here, so it's self-describing?	chipx86	Feb. 27, 2018, 7:51 p.m.
It seems that DiffSetManager.create_from_data() and DiffCommitManager.create_from_data() do primarily the same thing, with the exception that the latter creates and passes …	chipx86	Feb. 27, 2018, 8:10 p.m.
Can you use a keyword argument here?	chipx86	Feb. 27, 2018, 7:51 p.m.
Keyword arguments would also be great here.	chipx86	Feb. 27, 2018, 7:52 p.m.
This is the manager for DiffSet. I'm not sure DiffCommit is right here?	chipx86	Feb. 27, 2018, 7:53 p.m.
F841 local variable 'filediffs' is assigned to but never used	reviewbot	Feb. 27, 2018, 8:13 p.m.
Other such functions say "Subclasses." Would be nice to be consistent.	chipx86	April 3, 2018, 5:15 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	April 19, 2018, 6:25 p.m.
Swap these.	chipx86	May 1, 2018, 4:01 p.m.
Should be "BaseDiffManager", to match other base classes.	chipx86	May 1, 2018, 4:01 p.m.
Missing an entry for **kwargs.	chipx86	May 1, 2018, 4:02 p.m.
I think instead of copying all the keyword arguments from create_from_data, we just document **kwargs and say these are passed …	chipx86	May 1, 2018, 4:03 p.m.
No u prefix. Also missing docs for this argument.	chipx86	May 1, 2018, 4:04 p.m.
Missing docs for this argument.	chipx86	May 31, 2018, 6:20 p.m.
Missing a period.	chipx86	May 1, 2018, 4:05 p.m.
This says optional, but the argument doesn't have a default.	chipx86	May 1, 2018, 4:05 p.m.
This is in the wrong spot with regards to the argument list.	chipx86	May 1, 2018, 4:05 p.m.
Why don't we pass basedir anymore?	chipx86	June 14, 2018, 6:21 p.m.
Missing docs for **kwargs.	chipx86	May 1, 2018, 4:06 p.m.
"F" before "P".	chipx86	July 10, 2018, 7:56 p.m.
, optional	chipx86	July 10, 2018, 7:57 p.m.
Missing backtick and period.	chipx86	July 10, 2018, 7:57 p.m.
The ( should be on the next line. Kind of looks weird after another paren though.	chipx86	July 10, 2018, 7:58 p.m.
Blank line between these.	chipx86	July 10, 2018, 7:58 p.m.
F401 'reviewboard.diffviewer.features.dvcs_feature' imported but unused	reviewbot	July 24, 2018, 3:48 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/tests/test_diffcommit_manager.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F401 'nose' imported but unused
```

Commit:

-	ca421dfabef70a6ca3534379365ddb010e188ac1
+	56c8f10ea2d2c8461f2cc3dd358062094b2652f7

Diff:

Revision 2 (+696 -192)

Show changes

	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commit:

-	56c8f10ea2d2c8461f2cc3dd358062094b2652f7
+	0b5163d08571398f0e333492e06c5076e1c4bc09

Diff:

Revision 3 (+725 -194)

Show changes

	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/tests/test_diffset_manager.py (Diff revision 3)
The issue has been resolved. Show all issues
```
F841 local variable 'history' is assigned to but never used
```
reviewboard/diffviewer/tests/test_diffset_manager.py (Diff revision 3)
The issue has been resolved. Show all issues
```
W391 blank line at end of file
```

Commit:

-	0b5163d08571398f0e333492e06c5076e1c4bc09
+	72796b3bb31275b5fcf450bfb2ae35e2c3b0a314

Diff:

Revision 4 (+723 -194)

Show changes

	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/managers.py (Diff revision 2)
The issue has been resolved. Show all issues
```
typo: reivewboard
```
reviewboard/diffviewer/managers.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing a period.
```
reviewboard/diffviewer/managers.py (Diff revision 2)
The issue has been resolved. Show all issues
```
) should be on the line before.
```
reviewboard/diffviewer/managers.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Docstring?
```
reviewboard/diffviewer/managers.py (Diff revision 2)
The issue has been resolved. Show all issues
```
There are so many args here, let's put one per line.
```
reviewboard/diffviewer/managers.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Can we put one arg per line?
```
reviewboard/diffviewer/tests/test_diffcommit_manager.py (Diff revision 2)
The issue has been resolved. Show all issues
```
) should be on the previous line and no trailing comma after revision=1
```

Commit:

-	72796b3bb31275b5fcf450bfb2ae35e2c3b0a314
+	42dd02bbb4fb76a92d541e2484dea7a3629eb311

Diff:

Revision 5 (+751 -198)

Show changes

	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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

The issue has been resolved. Show all issues

While here, can you make request default to None and be optional? It's used for an eventual call to get_file_exists(), which doesn't require it. That will make it easier for testing work (I've hit this before while going through some complex diff testing for interdiffs).

reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
"will be empty"
```

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

The issue has been resolved. Show all issues

Can you pass these as keyword arguments? It'll be easier for maintenance.

reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
"filenames"
```
reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
"filename"
```
reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
"filename"
```
reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
"filenames"
```
reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Can you fix the indentation while you're here?
```
reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Missing a docstring.
```
reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Missing a docstring.
```

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

The issue has been dropped. Show all issues

This read better the way it was before.

brennie Feb. 27, 2018, 8:10 p.m.

The way it was before is no longer true.

chipx86 March 9, 2018, 4:12 p.m.

I meant the version without bullet points. The bullet points reads weird.

reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
It's not valid to put more text here between sections.
```

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

The issue has been resolved. Show all issues

Can you revert this? It doesn't simplify the code any, and makes it less future-proof. Better to have one task per line here.

reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Like above, can we make this optional?
```

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

The issue has been resolved. Show all issues

Is this necessary, given that this is a new manager for a new model?
If it is, we should document why in a comment here, and this should specify the right function and class name.

brennie Feb. 27, 2018, 8:10 p.m.
```
Accidental copy-paste refactor.
```

reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Can you pass a keyword argument here, so it's self-describing?
```

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

The issue has been resolved. Show all issues

It seems that DiffSetManager.create_from_data() and DiffCommitManager.create_from_data() do primarily the same thing, with the exception that the latter creates and passes a DiffCommit. It'd be nice to consolidate as much of this functionality as possible. I imagine the base class could have its own implementation that DiffCommitManager could override, calling the parent with a diff_commit= argument.

brennie Feb. 27, 2018, 8:10 p.m.

The latter also doesn't create a DiffSet.
I've extracted more behaviour into DiffProcessor.create_filediffs (which at this point should be a function since its a giant code smell) but the arguments and models are just different enough to make further refactoring ugly.

reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Can you use a keyword argument here?
```
reviewboard/diffviewer/managers.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Keyword arguments would also be great here.
```

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

The issue has been dropped. Show all issues

This is the manager for DiffSet. I'm not sure DiffCommit is right here?

brennie Feb. 27, 2018, 8:10 p.m.

This is describing why the function exists in the first place.

Change Summary:

Addressed Christian's issues.

Commit:

-	42dd02bbb4fb76a92d541e2484dea7a3629eb311
+	fe865810a504254c78b57518da41cc185a53d915

Diff:

Revision 6 (+757 -201)

Show changes

	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/managers.py (Diff revision 6)
The issue has been resolved. Show all issues
```
F841 local variable 'filediffs' is assigned to but never used
```

Change Summary:

Lint.

Commit:

-	fe865810a504254c78b57518da41cc185a53d915
+	3892e701afac0aa452130fdd5f1fdb66af7179aa

Diff:

Revision 7 (+757 -201)

Show changes

	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Fixed unit test failure.

Commit:

-	3892e701afac0aa452130fdd5f1fdb66af7179aa
+	4a608722445de17d05cfb24f185f66f1024db181

Diff:

Revision 8 (+756 -201)

Show changes

	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/managers.py (Diff revision 8)

Just realized this is going to be confusing with the other processor classes that are coming (for chunk/opcode processing). Maybe FileDiffsCreator.

It'd also be nice to move this to its own class, since this file's getting big otherwise.

chipx86 March 9, 2018, 4:57 p.m.

Oops, meant to open an issue here. Think I bumped the keyboard shortcut to toggle.

reviewboard/diffviewer/managers.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Other such functions say "Subclasses." Would be nice to be consistent.
```

Change Summary:

Refactor + address issues.

Description:

		The main logic of the `DiffSetManager` has been refactored into two
~		classes:
	~	parts:

~		`DiffProcessor`, which handles the actual processing of diff files into `FileDiff`s; and
	~	`reviewboard.diffviewer.filediff_creator`, which handles the actual processing of diff files into `FileDiff`s; and
		`DiffManagerBase`, which provides a base implementation of `create_from_upload` for both the `DiffSetManager` and the `DiffCommitManager`.

		The new `DiffCommitManager` is able to create `DiffCommit` objects from
		either uploaded files or from data, just as the `DiffSetManager` can for
		`DiffSet` objects.

		This patch also adds `DiffSetManager.crete_empty` to create empty
		DiffSets which will contain `DiffCommits`.

Commit:

-	4a608722445de17d05cfb24f185f66f1024db181
+	d4105b27bae9a276d23113124d95f09bae9a238a

Diff:

Revision 9 (+801 -211)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 timed out)

flake8 timed out.

JSHint timed out.

Change Summary:

Rebase

Commit:

-	d4105b27bae9a276d23113124d95f09bae9a238a
+	0a91cb4e9b0c3ccf0478e4396aa0e72be96464ac

Diff:

Revision 10 (+801 -211)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/filediff_creator.py (Diff revision 10)
The issue has been resolved. Show all issues
```
E126 continuation line over-indented for hanging indent
```

Change Summary:

Fixups

Description:

		The main logic of the `DiffSetManager` has been refactored into two
		parts:

		`reviewboard.diffviewer.filediff_creator`, which handles the actual processing of diff files into `FileDiff`s; and
		`DiffManagerBase`, which provides a base implementation of `create_from_upload` for both the `DiffSetManager` and the `DiffCommitManager`.

		The new `DiffCommitManager` is able to create `DiffCommit` objects from
		either uploaded files or from data, just as the `DiffSetManager` can for
		`DiffSet` objects.

~		This patch also adds `DiffSetManager.crete_empty` to create empty
	~	This patch also adds `DiffSetManager.create_empty` to create empty
		DiffSets which will contain `DiffCommits`.

Commit:

-	0a91cb4e9b0c3ccf0478e4396aa0e72be96464ac
+	482efd1a628218f93fd31e8ef0944517fa8a3ef8

Diff:

Revision 11 (+800 -211)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Do not pass basedir and base_commit_id to DiffCommitManager.create_from_data. fixes unit test failures in next patch

Commit:

-	482efd1a628218f93fd31e8ef0944517fa8a3ef8
+	d356972c7478fffdee78d209a1d7d84460e578b7

Diff:

Revision 12 (+808 -221)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
Swap these.
```
reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
Should be "BaseDiffManager", to match other base classes.
```
reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
Missing an entry for **kwargs.
```

reviewboard/diffviewer/managers.py (Diff revision 12)

The issue has been resolved. Show all issues

I think instead of copying all the keyword arguments from create_from_data, we just document **kwargs and say these are passed to create_from_data. Makes it easier to maintain and prevents discrepencies.

reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
No u prefix.
Also missing docs for this argument.
```
reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
Missing docs for this argument.
```
reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
Missing a period.
```
reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
This says optional, but the argument doesn't have a default.
```
reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
This is in the wrong spot with regards to the argument list.
```
reviewboard/diffviewer/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
Missing docs for **kwargs.
```

Change Summary:

Addressed Christian's feedback.

Commit:

-	d356972c7478fffdee78d209a1d7d84460e578b7
+	cd5fe6e71269fd9fb4d55db20b56623e9675f99c

Diff:

Revision 13 (+778 -224)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Do not pass diffset_history to DiffCommitManager

Commit:

-	cd5fe6e71269fd9fb4d55db20b56623e9675f99c
+	4c79653909cc7459358efb3bb9314f5f615e59bf

Diff:

Revision 14 (+775 -228)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

diff_commit_count -> diffcommit_count

Commit:

-	4c79653909cc7459358efb3bb9314f5f615e59bf
+	5bb04731dfb5644076b92b08b74c9c808896c3b8

Diff:

Revision 15 (+775 -228)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diff_commit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/managers.py (Diff revisions 12 - 15)

The issue has been dropped. Show all issues

Why don't we pass basedir anymore?

brennie June 14, 2018, 6:21 p.m.

The basedir is associated with the diffset and must be the repo root for a multi-commit RR. The diffcommit model doesnt store the basedir.

Commit:

-	5bb04731dfb5644076b92b08b74c9c808896c3b8
+	498b84b80e76ced5620abf3ac5621f5d3eb66364

Diff:

Revision 16 (+790 -230)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/evolutions/diffcommit_relations.py
	reviewboard/diffviewer/models/diffcommit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/filediff_creator.py (Diff revision 16)
The issue has been resolved. Show all issues
```
"F" before "P".
```
reviewboard/diffviewer/filediff_creator.py (Diff revision 16)
The issue has been resolved. Show all issues
```
, optional
```
reviewboard/diffviewer/filediff_creator.py (Diff revision 16)
The issue has been resolved. Show all issues
```
Missing backtick and period.
```
reviewboard/diffviewer/filediff_creator.py (Diff revision 16)
The issue has been resolved. Show all issues
```
The ( should be on the next line. Kind of looks weird after another paren though.
```
reviewboard/diffviewer/filediff_creator.py (Diff revision 16)
The issue has been resolved. Show all issues
```
Blank line between these.
```

Change Summary:

Addressed Christian's feedback

Commit:

-	498b84b80e76ced5620abf3ac5621f5d3eb66364
+	0d4bf895516e1e6861d346f979299436f6d498a7

Diff:

Revision 17 (+779 -230)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diffcommit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Depends On:

+	~~10079 - Correctly refer commit_message in DiffCommit.summary~~

Change Summary:

Fix create_filediffs() to save file counts.

Commit:

-	0d4bf895516e1e6861d346f979299436f6d498a7
+	2e9a2fda87df8e434419b0fd9dd4b197d0f1a0e1

Diff:

Revision 18 (+897 -232)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diffcommit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py
	reviewboard/diffviewer/tests/test_filediff_creator.py
	reviewboard/testing/testcase.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/tests/test_filediff_creator.py (Diff revision 18)
The issue has been resolved. Show all issues
```
F401 'reviewboard.diffviewer.features.dvcs_feature' imported but unused
```

Change Summary:

dev/release-4.0.x/dvcs/manager

Commit:

-	2e9a2fda87df8e434419b0fd9dd4b197d0f1a0e1
+	bc4b0ce7d27392e427cde2aeed65a1374146ab9e

Diff:

Revision 19 (+896 -232)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models/diffcommit.py
	reviewboard/diffviewer/tests/test_diffcommit_manager.py
	reviewboard/diffviewer/tests/test_diffset_manager.py
	reviewboard/diffviewer/tests/test_filediff_creator.py
	reviewboard/testing/testcase.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (f8f584a)