Post-commit workflow

Review Request #1737 — Created Aug. 16, 2010 and discarded

Information

Review Board

Reviewers

This patch introduces the notion of incremental reviews or reviewing as a whole is using the already implemented diff versioning system to represent all intermediate commits that make a file, providing the ability to implement a post-commit workflow. On a post-commit workflow the code to be reviewed was already committed, but not yet integrated into the mainstream code. That way, partial reviews can be performed up to the point were the code is good and finally merged.

If the user chooses to shift from pre-commit to a post-commit model we can make use of the patch versioning, to present a diff containing any number of commits. Those diffs, referred also as bundles, can be created from git using `git format-patch`. They are the standard way of submitting a patch to various projects, including the Linux Kernel and Git itself.

Currently when a new review is created on Review Board, it takes the uploaded patch, infer the oldest committed version referenced by the patch, also called base or left side, and applies the patch on top of it, creating a new version called head or right side. It then goes ahead and mark that patch as version one. If the user uploads a new patch, version two is created, and with that the possibility of seeing just what changed between the two versions, therefore reviewing the part, not the whole.

With the new workflow upon submitting a bundle to be reviewed, instead of just loading the submitted patch and marking it as version one, ReviewBoard infers the diff for every incremental revision from the oldest version present in the diff to the newest, apply it on top of the head and mark that as a version of the patch, in practice creating a version containing all changes from the base up to that point. It also stores information about which commits formed that partial patch to display later, such as the date of the commit and it's author.

There are a few limitations with this patch:
* Updating a diff implies in posting the whole bundle again. In incremental reviews only new content should be posted.
* The e-mail subsystem get's confused, as now multiple DiffSets can be created during the creation of the initial review. So it only sends an e-mail containing the info of the last patch in the bundle.
* The interface does not allow a way of interdiffs to be displayed, though the code is working. This is purely a GUI issue, and will be fixed soon.

Ran the standard tests, and two of them are failing:

* testPatchCRLFFileCRDiff
* testPatchFileWithFakeNoNewline

The code is correct. The test is now wrong (as I changed the semantics of the function patch())

Loading...