Change Summary:
Added a fix for closing private review requests.
Description: |
|
||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||
Depends On: |
|
||||||||||||||||||||||||
Commit: |
|
||||||||||||||||||||||||
Diff: |
Revision 2 (+138) |
Review Request #5596 — Created March 7, 2014 and submitted
Information | |
---|---|
anselina | |
Review Board | |
master | |
|
|
5653 | |
798641b... | |
Reviewers | |
reviewboard, students | |
This is a GitHub post-receive hook that will automatically close review requests as "submitted" after a push. To determine which review requests should be closed, it scans through each commit's commit message for the following strings (case-insensitive): "Reviewed at <reviewboard_url>/r/<id>" or "Review request #<id>". The regex used for this can be overriden in settings_local.py.
Added the post-receive webhook URL to a private GitHub repository, and tested different pushes:
- Commit with a review request ID that is not submitted (verified that the review request is closed and set to submitted)
- Commit with a review request ID that is already submitted (got a warning that the review request is already submitted)
- Commit without a review request ID in the commit message (got a debug message that no matching review request ID was found)
- Commit with a private review request (verified that the review request is closed)
- Commits referencing the same review request ID
- Multiple commits
- Merge commits
- Branch creation and deletion
- Commits pushed to two branchesI also overrode the regex and flags in settings_local.py, and got the expected change in matching review request IDs.
Description | From | Last Updated |
---|---|---|
Should probably be a @classmethod of HostingService in service.py which can be overriden by individual services like Github and Bitbucket. … |
B. b.ramnani | |
Same can be said about this method. This should probably be a private method. i.e. renamed to _get_review_id_to_commits_map... |
B. b.ramnani | |
Blank line at the end |
B. b.ramnani | |
A blank line below this last line. |
B. b.ramnani | |
We generally list full-package imports ahead of single-item imports, so the from one should go after import logging. |
|
|
How about we turn these into ints? |
|
|
There's an extra blank line here. |
|
|
There's an extra blank line here. |
|
|
Can you wrap this in parentheses instead of using the character? |
|
|
This function returns "http://example.com" on my localhost, when I tested for post-receive hook for the bitbucket repo. Is that the … |
B. b.ramnani |
Added a fix for closing private review requests.
Description: |
|
||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||
Depends On: |
|
||||||||||||||||||||||||
Commit: |
|
||||||||||||||||||||||||
Diff: |
Revision 2 (+138) |
Description: |
|
|||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||||||||||||||
Diff: |
Revision 3 (+144) |
|||||||||||||||||||||||||||||||||
Added Files: |
Description: |
|
|||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||
Diff: |
Revision 4 (+145) |
Made the regex customizable.
Summary: |
|
||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
||||||||||||||||||||||||||||||||||||
Testing Done: |
|
||||||||||||||||||||||||||||||||||||
Commit: |
|
||||||||||||||||||||||||||||||||||||
Diff: |
Revision 5 (+157) |
reviewboard/hostingsvcs/github.py (Diff revision 5) |
---|
Should probably be a @classmethod of HostingService in service.py which can be overriden by individual services like Github and Bitbucket. Would increase reusability. Would also increase encapsulation, since the repository_url_patterns need to reference this view. Maybe the mentors could comment on that.
reviewboard/hostingsvcs/github.py (Diff revision 5) |
---|
Same can be said about this method. This should probably be a private method. i.e. renamed to _get_review_id_to_commits_map...
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+157) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 7 (+159) |
Defined the hook URL in github.py instead (depends on /r/5510).
Depends On: |
|
||||
---|---|---|---|---|---|
Commit: |
|
||||
Diff: |
Revision 8 (+162) |
reviewboard/hostingsvcs/github.py (Diff revision 8) |
---|
We generally list full-package imports ahead of single-item imports, so the
from
one should go afterimport logging
.
Changed all review request ID variables to ints, and fixed style issues.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 9 (+158) |
reviewboard/settings.py (Diff revision 9) |
---|
Can you wrap this in parentheses instead of using the character?
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 10 (+157) |
Added a try/except block to make sure the payload is in JSON format before processing it (since GitHub also supports x-www-form-urlencoded).
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 11 (+162) |
reviewboard/hostingsvcs/hook_utils.py (Diff revision 11) |
---|
This function returns "http://example.com" on my localhost, when I tested for post-receive hook for the bitbucket repo. Is that the expected behavior ?