Summary

Fixes associated to Issue 1345: post-review does not process a pending Perforce changeset in v0.2 beta3 (Note that it is a regression from v0.2 beta2)

Review Request #1157 — Created Oct. 6, 2009 and submitted

Information

Owner

lonico

Repository

RBTools

Branch

Bugs

1345

Depends On

Reviewers

Groups

rbtools, reviewboard

People

Description

Fixes associated to Issue 1345: post-review does not process a *pending* Perforce changeset in v0.2 beta3 (Note that it is a regression from v0.2 beta2)

Also 2 changes that are not strictly related:
1) allow *pending* to be on the second line in addition to the first line
2) append the list of bugs to existing list if it exists already rather than
overwritting it.

Note the "view diffs" view does not show a white space change on line 1586.  I fixed the indentation, otherwise the for loop was ending after the first line.

Testing Done

Tested post-review against our perforce repository, using two *pending* changeset.
Tried combination of with and without -r to find existing review based on changeset number or review id.
Added bug list to check append feature.


Checked for different values of --bugs-closed.  After each invocation, checked the GUI for a correct list, and then submitted the review before the next try:

1) No option

Bugs: empty

2) --bugs-closed=123

Bugs: 123

3) --bugs-closed=456

Bugs: 123, 456

4) --bugs-closed=456 (repeat)

Bugs: 123, 456

5) --bugs-closed=12

Bugs: 12, 123, 456

6) --bugs-closed=1234

Bugs: 12, 123, 456, 1234

7) --bugs-closed="1234, 12345, 1,"

Bugs: 1, 12, 123, 456, 1234, 12345

8) --bugs-closed="1,,   234, 12345, 1, ,,"

Bugs: 1, 123, 234, 456, 1234, 12345

10) --bugs-closed="  ,,   ,,1,,   234, 12345, 1, ,, , , ,,,,   ,,,,,"

Bugs: 1, 12, 123, 234, 456, 1234, 12345


Checked the database: Bugs field: 1234,12,456,1,123,234,12345

rbtools/postreview.py (Diff revision 1)

Note there is a whitespace change that is not shown at line 1586.

I tried to toggle the "show whitespace changes" in RB but without results.

Description:

		Fixes associated to Issue 1345: post-review does not process a pending Perforce changeset in v0.2 beta3 (Note that it is a regression from v0.2 beta2)

		Also 2 changes that are not strictly related:
		1) allow pending to be on the second line in addition to the first line
		2) append the list of bugs to existing list if it exists already rather than
		overwritting it.
	+
	+	Note the "view diffs" view does not show a white space change on line 1586. I fixed the indentation, otherwise the for loop was ending after the first line.

Groups:

+	rbtools

One of the version fixes went in already, so you'll want to update your review request along with the change below.

rbtools/postreview.py (Diff revision 1)

If the review request references bug 12, and I pass in the value 123, this will fail.

I think what we want to do is split options.bugs_closed into an array, then create a set() of that array and review_request['bugs_closed'], and then join that back into a string to pass into set_review_request_field.

Change Summary:

New set of diffs based on Chris's feedback

1) One of the index issue was solved by a previous commit
2) Changed the logic to append new bug ids (using a set)
3) Note that there is a white space change on line 1586.  I fixed the indentation, otherwise the for loop was ending after the first line.

Testing Done:

		Tested post-review against our perforce repository, using two pending changeset.
		Tried combination of with and without -r to find existing review based on changeset number or review id.
		Added bug list to check append feature.
	+
	+
	+
	+	Checked for different values of --bugs-closed. After each invocation, checked the GUI for a correct list, and then submitted the review before the next try:
	+
	+	1) No option
	+
	+	Bugs: empty
	+
	+	2) --bugs-closed=123
	+
	+	Bugs: 123
	+
	+	3) --bugs-closed=456
	+
	+	Bugs: 123, 456
	+
	+	4) --bugs-closed=456 (repeat)
	+
	+	Bugs: 123, 456
	+
	+	5) --bugs-closed=12
	+
	+	Bugs: 12, 123, 456
	+
	+	6) --bugs-closed=1234
	+
	+	Bugs: 12, 123, 456, 1234
	+
	+	7) --bugs-closed="1234, 12345, 1,"
	+
	+	Bugs: 1, 12, 123, 456, 1234, 12345
	+
	+	8) --bugs-closed="1,, 234, 12345, 1, ,,"
	+
	+	Bugs: 1, 123, 234, 456, 1234, 12345
	+
	+	10) --bugs-closed=" ,, ,,1,, 234, 12345, 1, ,, , , ,,,, ,,,,,"
	+
	+	Bugs: 1, 12, 123, 234, 456, 1234, 12345
	+
	+
	+
	+	Checked the database: Bugs field: 1234,12,456,1,123,234,12345

Diff:

Revision 2 (+12 -8)

Show changes

rbtools/postreview.py

rbtools/postreview.py (Diff revision 2)
```
comment on line 1564
```
rbtools/postreview.py (Diff revision 2)
```
comment on line 1565
```

Ship it!

Looks good. Committed to master as 84f17d2.