PrProcessor » History » Version 6
Dominic Cleal, 10/13/2016 06:19 AM
action on failed PR review
1 | 1 | Dominic Cleal | h1. PR Processor |
---|---|---|---|
2 | |||
3 | prprocessor is a web service hosted on the project's OpenShift account, which receives GitHub webhooks/notifications and helps sync PRs to Redmine and Jenkins. |
||
4 | |||
5 | It's a simple Sinatra app, and the source is available here: https://github.com/theforeman/prprocessor. |
||
6 | |||
7 | h2. OpenShift |
||
8 | |||
9 | It runs on OpenShift using a group account, to which a few developers have push access to. When merging into the GitHub repository, please keep this in sync with OpenShift's repo by pushing to both. |
||
10 | |||
11 | h2. GitHub integration |
||
12 | |||
13 | It receives requests from GitHub when pull requests are opened or synchronized (updated). These contain lots of info about the PR, including the author, branches etc. A repository admin needs to add the webhook to send these events - more information on the "Jenkins":http://projects.theforeman.org/projects/foreman/wiki/Jenkins#PR-scanner-repo-hooks wiki page. |
||
14 | |||
15 | 2 | Dominic Cleal | It sets labels and performs checks on the PR to ensure it meets our usual standards. |
16 | 1 | Dominic Cleal | |
17 | 2 | Dominic Cleal | h3. Labels |
18 | |||
19 | When a PR is first opened, it sets: |
||
20 | |||
21 | * _Not yet reviewed_ |
||
22 | * _Needs testing_ |
||
23 | |||
24 | 6 | Dominic Cleal | When a "needs changes" (rejected) PR review is filed, it: |
25 | |||
26 | * Removes any _Not yet reviewed_ or _Needs re-review_ labels |
||
27 | * Sets _Waiting on contributor_ |
||
28 | |||
29 | 2 | Dominic Cleal | When a PR is updated, it: |
30 | |||
31 | * Removes any _Waiting on contributor_ label |
||
32 | 3 | Dominic Cleal | * Sets _Needs testing_ |
33 | 2 | Dominic Cleal | * If the _Not yet reviewed_ label is missing, it sets _Needs re-review_ |
34 | |||
35 | This encourages the workflow for reviewers to remove the _Not yet reviewed_ label when they've made the first review. When the PR is updated, it gets _Needs re-review_ so they can attend to it sooner, before starting on new PRs. |
||
36 | |||
37 | 4 | Lukas Zapletal | When a PR is not updated for six months with _Waiting on contributor_ label, it: |
38 | |||
39 | * Adds a comment |
||
40 | * Sets _Inactive_ label |
||
41 | * Closes the PR |
||
42 | |||
43 | Every week, maximum of five inactive pull requests are closed. |
||
44 | |||
45 | 1 | Dominic Cleal | h3. Commit message styles |
46 | |||
47 | 5 | Dominic Cleal | For repositories in "repos.yaml":https://github.com/theforeman/prprocessor/blob/master/config/repos.yaml where @redmine_required@ is true, it enforces that: |
48 | 1 | Dominic Cleal | |
49 | # Commit messages match the style in [[Reviewing_patches-commit_message_format]] |
||
50 | # A Redmine ticket is referenced in the commit message |
||
51 | |||
52 | 2 | Dominic Cleal | If it fails this check, it will add the _Waiting on contributor_ label and leave a comment. |
53 | |||
54 | 1 | Dominic Cleal | h3. PR is mergeable |
55 | |||
56 | It checks whether the PR is mergeable against the branch it's proposed for. This will only pick up this error when the PR is updated because the service is event-driven, so it won't notify authors when a PR goes out of sync due to master/develop being changed, and the PR is not updated. |
||
57 | 2 | Dominic Cleal | |
58 | If it fails this check, it will replace any waiting review labels with the _Waiting on contributor_ label and leave a comment. |
||
59 | 1 | Dominic Cleal | |
60 | h2. Redmine integration |
||
61 | |||
62 | When it receives an event, the corresponding Redmine ticket is updated to link it to the pull request. The script updates these fields: |
||
63 | |||
64 | # _Status_ is changed to _Ready for Testing_ |
||
65 | # _Assigned To_ is changed to the PR author - update "users.yaml":https://github.com/theforeman/prprocessor/blob/master/config/users.yaml for new or unknown developers |
||
66 | # _Target version_ (usually sprints) is set to the latest available version |
||
67 | # _Pull request_ is set to the URL of the PR |
||
68 | 5 | Dominic Cleal | |
69 | The repositories listed in repos.yaml will also be synchronised on the Redmine instance using an hourly cronjob, and Redmine will associate commits and close tickets based on what's merged. Use "fixes #99999" to add the commit and close the ticket or "refs #99999" to only add the commit. |
||
70 | 1 | Dominic Cleal | |
71 | h2. Jenkins integration |
||
72 | |||
73 | A "PR scanner" job runs regularly on Jenkins and is responsible for all checking, testing and updating of pull requests. Since it uses a lot of API queries, it only runs every 30 minutes. More about this job is on the "Jenkins wiki page":http://projects.theforeman.org/projects/foreman/wiki/Jenkins#PR-scanner. |
||
74 | |||
75 | To get faster PR testing, when a PR hook is received by the prprocessor, it triggers an immediate run/build of the Jenkins scanner job against the relevant PR. No further action is taken by the prprocessor. |