Summary

[WIP] Add support for multi-commit review requests.

Review Request #6618 — Created Nov. 21, 2014 and discarded Jan. 6, 2015, 11:02 a.m.

Information

Owner

brennie

Repository

Review Board

Branch

master

Bugs

Depends On

~~6642~~

Reviewers

Groups

reviewboard, students

People

Description

Add the a diffviewer.Commit model. Each Commit belongs to a
DiffSet (which may have many commits) and may have many FileDiffs.
However, an old-fashioned review request (of squashed changes) will
not have any Commits associated with it.

Add a foreign key on the FileDiff model which links it to a
Commit. Every FileDiff still corresponds to a DiffSet, but they
now can optionally correspond to a Commit.

Add a RelationCounterField on the DiffSet model to keep track of
the number of Commits corresponding to the DiffSet. This way, the
DiffSet can easily be checked if it is a multi-commit review request
or not.

Add evolution (new_commit_fields.py) for new fields on FileDiff
and DiffSet.

TODO:
- Add an API endpoint so that multiple commits can be attached a review
request

Testing Done

Issues

Description	From	Last Updated
Col: 38 W292 no newline at end of file	reviewbot	Nov. 21, 2014, 11:37 p.m.
Two blank lines.	chipx86	Nov. 24, 2014, 5:49 p.m.
We use whatever_count instead of num_whatevers for all other counter fields, so we should stick with that for this one.	chipx86	Nov. 24, 2014, 5:48 p.m.
We usually swap this around to be named like COMMIT_TYPE_MERGE and COMMIT_TYPE_CHANGE.	chipx86	Nov. 24, 2014, 5:47 p.m.
There are some crucial differences between how TEXT (TextField) and VARCHAR(x) (CharField(max_length=x)) columns are stored. If we ever want to …	chipx86	Nov. 24, 2014, 5:47 p.m.
This should take a choices= argument that limits it to one of the commit types. It helps with validation and …	chipx86	Nov. 24, 2014, 5:48 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Jan. 5, 2015, 4:09 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py

reviewboard/diffviewer/models.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 38
 W292 no newline at end of file
```

Change Summary:

Formatting.

Commit:

f3a0c3cd25c2a0ab2ff8a3127237dab1ed49ce21

0d1b6b485a308a03ae25d6874f50522165407aa2

Diff:

Revision 2 (+48 -1)

Show changes

	reviewboard/diffviewer/models.py
	reviewboard/diffviewer/evolutions/__init__.py
	reviewboard/diffviewer/evolutions/add_commit_fields.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py



Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py

reviewboard/diffviewer/evolutions/add_commit_fields.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Two blank lines.
```

reviewboard/diffviewer/models.py (Diff revision 2)

The issue has been resolved. Show all issues

We use whatever_count instead of num_whatevers for all other counter fields, so we should stick with that for this one.

reviewboard/diffviewer/models.py (Diff revision 2)

The issue has been resolved. Show all issues

We usually swap this around to be named like COMMIT_TYPE_MERGE and COMMIT_TYPE_CHANGE.

reviewboard/diffviewer/models.py (Diff revision 2)

The issue has been resolved. Show all issues

There are some crucial differences between how TEXT (TextField) and VARCHAR(x) (CharField(max_length=x)) columns are stored. If we ever want to do any queries on authors or committers (I can see us wanting to do that), then we're going to want to use a CharField for performance.
I think it's safe to say that an author or committer will always be a reasonable size (< 256 chars, probably < 128 chars). Given that, I think we can use a CharField here.

reviewboard/diffviewer/models.py (Diff revision 2)

The issue has been resolved. Show all issues

This should take a choices= argument that limits it to one of the commit types. It helps with validation and display in the admin UI.

Change Summary:

Depend on r/6642. Evolution cleanly applies. Fix Christian's issues.

Description:

		Add the a `diffviewer.Commit` model. Each `Commit` belongs to a
		`DiffSet` (which may have many commits) and may have many `FileDiff`s.
		However, an old-fashioned review request (of squashed changes) will
		not have any `Commit`s associated with it.

		Add a foreign key on the `FileDiff` model which links it to a
		`Commit`. Every `FileDiff` still corresponds to a `DiffSet`, but they
		now can optionally correspond to a `Commit`.

		Add a `RelationCounterField` on the `DiffSet` model to keep track of
		the number of `Commits` corresponding to the `DiffSet`. This way, the
		`DiffSet` can easily be checked if it is a multi-commit review request
		or not.

		Add evolution (`new_commit_fields.py`) for new fields on `FileDiff`
		and `DiffSet`.

-		NB: The evolution does not currently cleanly apply because of a bug in
-		django-evolution.
-
		TODO:
		- Add an API endpoint so that multiple commits can be attached a review
		request

Depends On:

~~6642 - Make RelationCounterField's rel_field_name default to None~~

Commit:

0d1b6b485a308a03ae25d6874f50522165407aa2

f8cfd3d0026c569ab262820b9b888fe5c1d5634e

Diff:

Revision 3 (+54 -1)

Show changes

	reviewboard/diffviewer/models.py
	reviewboard/diffviewer/evolutions/__init__.py
	reviewboard/diffviewer/evolutions/add_commit_fields.py

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py

Change Summary:

Fix model

Commit:

f8cfd3d0026c569ab262820b9b888fe5c1d5634e

f961b3b22921e2baa84bd6cc30e6234dc8651556

Diff:

Revision 4 (+54 -1)

Show changes

	reviewboard/diffviewer/models.py
	reviewboard/diffviewer/evolutions/__init__.py
	reviewboard/diffviewer/evolutions/add_commit_fields.py

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py

Change Summary:

Add commit_ordinal field to allow commits to be ordered within a DiffSet

Commit:

f961b3b22921e2baa84bd6cc30e6234dc8651556

785dc534b222aa080108ea49fdb3ba5baa847228

Diff:

Revision 5 (+60 -1)

Show changes

	reviewboard/diffviewer/models.py
	reviewboard/diffviewer/evolutions/__init__.py
	reviewboard/diffviewer/evolutions/add_commit_fields.py

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/diffviewer/evolutions/__init__.py

Change Summary:

Work on commit endpoint

Commit:

785dc534b222aa080108ea49fdb3ba5baa847228

8dd0fb9b6771704d7ea82b01b4dfd512bae67c4b

Diff:

Revision 6 (+92 -3)

Show changes

	reviewboard/diffviewer/models.py
	reviewboard/diffviewer/evolutions/__init__.py
	reviewboard/diffviewer/evolutions/add_commit_fields.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/commit.py

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/webapi/resources/__init__.py
    reviewboard/diffviewer/evolutions/__init__.py
    reviewboard/webapi/resources/commit.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/models.py
    reviewboard/diffviewer/evolutions/add_commit_fields.py
    reviewboard/webapi/resources/__init__.py
    reviewboard/diffviewer/evolutions/__init__.py
    reviewboard/webapi/resources/commit.py

reviewboard/webapi/resources/commit.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

Status:: Discarded