Summary

Status integration for pull requests

Review Request #8463 — Created Oct. 16, 2016 and discarded Oct. 26, 2021, 12:16 p.m.

Information

Owner

finaiized

Repository

Review Board

Branch

dvcs

Bugs

Depends On

Blocks

8547

Reviewers

Groups

reviewboard, students

People

Description

Adds support for setting statuses on pull requests.

When a new pull request is created, and that repository is linked to a RB webhook, a new corresponding review request will be created. That will automatically set the pending status for the pull request.

Whenever an issue is open on the review request, the status is set to "error".
When the review request is "approved" (defaulting to 1+ ship it's and no open issues), the status is set to "success".
Otherwise, the PR has a "pending" status.

Testing Done

Added unit tests
- to verify the pull request hook is called, and only works with pull_request events and the right signature
- to verify pull requests are created and updated when a pull request is created
- to verify the pull request's status starts off with 'pending', is 'error' when there's open issues, and 'success' when approved

Manual testing
- Created a pull request on a repository linked to ReviewBoard through webhooks
- Verified the review request was created with 'pending' status
- Verified that opening an issue on the review request sets the 'error' status on the pull request
- Verified that having no open issue and adding a 'Ship it!' sets the 'success' status on the pull request

Issues

Description	From	Last Updated
'ipdb' imported but unused	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
local variable 'e' is assigned to but never used	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (112 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
'ipdb' imported but unused	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (97 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
local variable 'e' is assigned to but never used	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
'ipdb' imported but unused	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (97 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
local variable 'e' is assigned to but never used	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
local variable 'pull_request_id' is assigned to but never used	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (108 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 9 E266 too many leading '#' for block comment	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
'ipdb' imported but unused	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Nov. 15, 2016, 5:06 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (97 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
local variable 'e' is assigned to but never used	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (115 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 9 E266 too many leading '#' for block comment	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (115 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (108 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (181 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (87 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
'ipdb' imported but unused	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (97 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
local variable 'e' is assigned to but never used	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (115 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (115 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (108 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (181 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (87 > 79 characters)	reviewbot	Nov. 15, 2016, 5:07 p.m.
Col: 80 E501 line too long (98 > 79 characters)	reviewbot	Nov. 15, 2016, 5:10 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:10 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:10 p.m.
Col: 80 E501 line too long (97 > 79 characters)	reviewbot	Nov. 15, 2016, 5:10 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:10 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 15, 2016, 5:13 p.m.
local variable 'e' is assigned to but never used	reviewbot	Nov. 15, 2016, 5:13 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:13 p.m.
Col: 80 E501 line too long (115 > 79 characters)	reviewbot	Nov. 15, 2016, 5:13 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:13 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:13 p.m.
Col: 80 E501 line too long (115 > 79 characters)	reviewbot	Nov. 15, 2016, 5:13 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:13 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Nov. 15, 2016, 5:13 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 15, 2016, 5:14 p.m.
Col: 80 E501 line too long (108 > 79 characters)	reviewbot	Nov. 15, 2016, 5:15 p.m.
Col: 80 E501 line too long (181 > 79 characters)	reviewbot	Nov. 15, 2016, 5:15 p.m.
Col: 80 E501 line too long (111 > 79 characters)	reviewbot	Nov. 15, 2016, 5:15 p.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Nov. 15, 2016, 5:15 p.m.
Col: 80 E501 line too long (87 > 79 characters)	reviewbot	Nov. 15, 2016, 5:15 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Nov. 15, 2016, 5:18 p.m.
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 15, 2016, 5:19 p.m.
Col: 80 E501 line too long (115 > 79 characters)	reviewbot	Nov. 15, 2016, 5:19 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Nov. 15, 2016, 5:20 p.m.
Col: 80 E501 line too long (111 > 79 characters)	reviewbot	Nov. 15, 2016, 5:22 p.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Nov. 15, 2016, 5:22 p.m.
Col: 80 E501 line too long (87 > 79 characters)	reviewbot	Nov. 15, 2016, 5:23 p.m.
Undo this.	brennie	Nov. 24, 2016, 10:26 a.m.
alphabetical order - i believe this should be on line 13	LA larmiej	Nov. 24, 2016, 10:34 a.m.
doc string	LA larmiej	Nov. 24, 2016, 10:35 a.m.
Full path	brennie	Nov. 24, 2016, 10:28 a.m.
Blank line between these. Same below. Also the docs for this param are insufficient.	brennie	Nov. 24, 2016, 10:29 a.m.
unicode	brennie	Nov. 24, 2016, 10:30 a.m.
unicode	brennie	Nov. 24, 2016, 10:30 a.m.
unicode. No Optional	brennie	Nov. 24, 2016, 10:30 a.m.
totally up to you to implement, but i will suggest that you can have something like this states = ['pending', …	LA larmiej	Nov. 24, 2016, 10:43 a.m.
Docstrings of the form: """Single line summary mutli-line description. """	brennie	Nov. 24, 2016, 10:31 a.m.
Docstring must be in the imperitive (i.e., "Return") mood.	brennie	Nov. 24, 2016, 10:31 a.m.
client.	brennie	Nov. 24, 2016, 10:31 a.m.
Use % formatting.	brennie	Nov. 24, 2016, 10:31 a.m.
function doc string	LA larmiej	Nov. 24, 2016, 10:44 a.m.
Put this inside the if. Missing a period.	brennie	Nov. 24, 2016, 10:31 a.m.
Pyt this inside the elif. Missing a period. A semicolon doesn't work here, either.	brennie	Nov. 24, 2016, 10:32 a.m.
Single quotes here and throughout.	brennie	Nov. 24, 2016, 10:33 a.m.
Should be alphabetical. Above from reviewboard.scmtools.core import Branch, Commit	CO Connor-Y	Dec. 3, 2016, 3 p.m.
Extra line, delete this.	CO Connor-Y	Dec. 3, 2016, 3:01 p.m.
I think this function needs more docstring info (Args, Returns...)	ST stensby	Dec. 3, 2016, 3:09 p.m.
Should this TODO should be done/removed?	ST stensby	Dec. 3, 2016, 3:03 p.m.
Should this TODO be done/removed?	ST stensby	Dec. 3, 2016, 3:03 p.m.
Extra line here.	CO Connor-Y	Dec. 3, 2016, 3:02 p.m.
i believe this should be review_request (reviewboard.scmtools.models.ReviewRequest): except this is different	LA larmiej	Dec. 3, 2016, 4:39 p.m.
'Review' imported but unused	reviewbot	Dec. 3, 2016, 4:42 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 3, 2016, 4:42 p.m.
Imperative mode, Return rather than Returns. I always make a mistake with this one, very tricky	LA larmiej	Dec. 10, 2016, 1:07 p.m.
'Review' imported but unused	reviewbot	Dec. 3, 2016, 4:44 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 3, 2016, 4:44 p.m.
'Review' imported but unused	reviewbot	Dec. 3, 2016, 4:44 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 3, 2016, 4:44 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 10, 2016, 1:13 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/builtin_fields.py
    reviewboard/hostingsvcs/github.py
    reviewboard/reviews/models/review_request.py
    reviewboard/hostingsvcs/service.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/builtin_fields.py
    reviewboard/hostingsvcs/github.py
    reviewboard/reviews/models/review_request.py
    reviewboard/hostingsvcs/service.py

reviewboard/hostingsvcs/github.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'ipdb' imported but unused
```
reviewboard/hostingsvcs/github.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/hostingsvcs/github.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 local variable 'e' is assigned to but never used
```
reviewboard/hostingsvcs/github.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (100 > 79 characters)
```
reviewboard/hostingsvcs/github.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (112 > 79 characters)
```
reviewboard/reviews/builtin_fields.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```

Change Summary:

Update pull request status every time a review request has a new review posted.

Commit:

5685c02cb52cbebdef551e8b712d56d2a9a344c8

ccce168758544ef7ad30c9bcf183974c67505e2a

Diff:

Revision 2 (+173 -5)

	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/service.py
	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/models/review.py
	reviewboard/reviews/models/review_request.py

		Adds support for setting statuses on pull requests.

		When a new pull request is created, and that repository is linked to a RB webhook, a new corresponding review request will be created. That will automatically set the pending status for the pull request.

~		In the future, different statuses would be set as well. For example, when an issue is opened on the review request, the PR status will be set to "error".
	~	Whenever an issue is open on the review request, the status is set to "error".
	+	When the review request is "approved" (defaulting to 1+ ship it's and no open issues), the status is set to "success".
	+	Otherwise, the PR has a "pending" status.

	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/service.py
	reviewboard/hostingsvcs/tests/test_github.py
	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/evolutions/__init__.py
	reviewboard/reviews/evolutions/pull_request.py
	reviewboard/reviews/models/base_comment.py
	reviewboard/reviews/models/review.py
	1 more

	+	Added unit tests
	+	to verify the pull request hook is called, and only works with `pull_request` events and the right signature
	+	to verify pull requests are created and updated when a pull request is created
	+	to verify the pull request's status starts off with 'pending', is 'error' when there's open issues, and 'success' when approved
	+	Manual testing
	+	Created a pull request on a repository linked to ReviewBoard through webhooks
	+	Verified the review request was created with 'pending' status
	+	Verified that opening an issue on the review request sets the 'error' status on the pull request
	+	Verified that having no open issue and adding a 'Ship it!' sets the 'success' status on the pull request

	reviewboard/diffviewer/diffutils.py
	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/service.py
	reviewboard/hostingsvcs/tests/test_github.py
	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/evolutions/__init__.py
	reviewboard/reviews/evolutions/pull_request.py
	reviewboard/reviews/models/base_comment.py
	2 more