Summary

Separate storing commit IDs and updating from them.

Review Request #5331 — Created Jan. 26, 2014 and submitted Jan. 29, 2014, 3:57 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Separate storing commit IDs and updating from them.

Review requests previously stored commit IDs and allowed setting them
during creation with the assumption being that they were intended for
setting the review request information and diff.

This change removes that assumption. Commit IDs can now be set on new
review requests or updated later on drafts. The old behavior is invoked
by passing a commit_id in the request along with create_from_commit_id=1
or update_from_commit_id=1 (depending on the operation). For
compatibility reasons, setting changenum will automatically trigger the
old behavior as well.

Since commit IDs are now stored along with drafts, we now record them in
change descriptions as well. Some changes needed to be made to field
recording logic for this. I also split up the existing CommitField
into ChangeField and CommitField in order to better separate out
responsibilities and provide a more appropriate label depending on
whether Perforce-style changesets are used vs. commit IDs.

This does remove the ability to update based on a commit ID using
HTTP PUT on the review request item resource, in favor of the draft API.

Testing Done

JavaScript and Python unit tests passed.

Posted an exiting commit using the New Review Request UI. Saw the commit
information in the draft, plus the summary, description, and diff.
Published and saw the commit information remain.

Updated RBTools to post the commit ID, and posted a new branch. It didn't
try to grab the review request info or diff from the server, but rather used
what I had. Saw the commit ID in the draft. Published it, then made a change
locally and posted again. Commit ID changed. Published, and saw the change
entry for the commit ID.

Posted a change on a Perforce repository using rbt post.
Saw the "Change:" field instead of the "Commit:" field. Published.
Posted an update with the same command, and it successfully reused and
updated my review request.

Issues

Description	From	Last Updated
Why are you changing some of these from commit_id to commit-id? If it's for consistency, can you change all to …	david	Jan. 29, 2014, 3:48 p.m.
Should this be 'commit-id'?	david	Jan. 29, 2014, 3:49 p.m.
Hmm. Shouldn't update() still have commit_id in its fields? Or is that handled by self.fields?	david	Jan. 29, 2014, 3:54 p.m.
Same question with - vs _	david	Jan. 29, 2014, 3:49 p.m.

Ship it!

Ship It!

chipx86 Jan. 27, 2014, 3:23 a.m.

This is a production server. Do not post any test messages of any sort here. You can use http://demo.reviewboard.org/ for that.

```
Ship it
```

In your testing done, you list "JavaScript and Python unit tests failed." Shouldn't they... not fail?

chipx86 Jan. 27, 2014, 5:28 p.m.

Hahaha, that's the typo of the day.

Succeeded. The unit tests succeeded.

chipx86 Jan. 27, 2014, 5:29 p.m.
```
(Or passed. Whatever.)
```

reviewboard/webapi/resources/review_request.py (Diff revision 1)

The issue has been dropped. Show all issues

Why are you changing some of these from commit_id to commit-id? If it's for consistency, can you change all to be commit-id and name your new field to create-from-commit-id ?

chipx86 Jan. 27, 2014, 5:29 p.m.

GET query parameters always use dashes. POST/PUT fields always use underscores. commit_id was the only query parameter using the wrong format.

reviewboard/webapi/resources/review_request.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Should this be 'commit-id'?
```
1. chipx86 Jan. 27, 2014, 5:29 p.m.
  This is a field, so it uses underscores. Same below.
reviewboard/webapi/resources/review_request_draft.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Same question with - vs _
```

Change Summary:: Passed, not failed.
Testing Done:: ~
JavaScript and Python unit tests failed.
~
JavaScript and Python unit tests passed.

Posted an exiting commit using the New Review Request UI. Saw the commit
information in the draft, plus the summary, description, and diff.
Published and saw the commit information remain.

Updated RBTools to post the commit ID, and posted a new branch. It didn't
try to grab the review request info or diff from the server, but rather used
what I had. Saw the commit ID in the draft. Published it, then made a change
locally and posted again. Commit ID changed. Published, and saw the change
entry for the commit ID.

~
Posted a change on a Perforce repository using rbt post <changenum>.
~
Posted a change on a Perforce repository using rbt post.
Saw the "Change:" field instead of the "Commit:" field. Published.
Posted an update with the same command, and it successfully reused and
updated my review request.

reviewboard/webapi/resources/review_request.py (Diff revision 1)

The issue has been dropped. Show all issues

Hmm. Shouldn't update() still have commit_id in its fields? Or is that handled by self.fields?

chipx86 Jan. 29, 2014, 3:50 p.m.

I've removed the ability to update it in the review request resource itself, and moved that to the draft, since commit_id now works and is treated like any other field.

I kept support for PUTting changenum on ReviewRequestResource for backwards-compatibility reasons, but since commit_id was introduced in 2.0, no need to keep that compatibility.

david Jan. 29, 2014, 3:53 p.m.
```
Fair enough.
```

Ship it!

```
Ship It!
```

Status:: Completed

~		JavaScript and Python unit tests failed.
	~	JavaScript and Python unit tests passed.

		Posted an exiting commit using the New Review Request UI. Saw the commit
		information in the draft, plus the summary, description, and diff.
		Published and saw the commit information remain.

		Updated RBTools to post the commit ID, and posted a new branch. It didn't
		try to grab the review request info or diff from the server, but rather used
		what I had. Saw the commit ID in the draft. Published it, then made a change
		locally and posted again. Commit ID changed. Published, and saw the change
		entry for the commit ID.

~		Posted a change on a Perforce repository using `rbt post <changenum>`.
	~	Posted a change on a Perforce repository using `rbt post`.
		Saw the "Change:" field instead of the "Commit:" field. Published.
		Posted an update with the same command, and it successfully reused and
		updated my review request.