openSUSE:Build Service Concept Review
Request Review System
OBS supports the review of requests. The plain request system is already enough for the use case where an unknown person wants to submit a change to a project or package. Any target project or package owner can review the request and approve or decline it.
In some cases (in esp. important projects) it is not enough when a single person is reviewing a request or this person is unsure and wants to ask another person before. Also some process may defines a multiple number of people or groups who needs to review changes. This review system is intended to cover this use case.
When a request gets a reviewer added the request changes from state "new" to "review". The reviewers will see the request in their "my requests" list visible in webui or osc and need to approve it. Whenever any reviewer is rejecting the review approval the entire request changes to "rejected". When all reviewers have approved the review the request goes back to "new" state.
Another use case is to do the (maybe time consuming) review of all requests first step by step and accept all request then with one step. So all source changes become affective at a defined point of time. This is usefull for large projects like openSUSE:Factory, where the review can happen during the week, but the entire rebuild will be triggered only once a week.
Request owners can still ignore the open review tasks but they get warned by the UI if they really want to ignore the feedback.
Set default reviewers for a project or package
This can be enforced by defining these reviewers as user or groups in the target project with the "reviewer" role. As a result a new request to this project or package will always created in "review" state initially.
Just use the webui or osc to add a user or a group with this role:
osc maintainer -r reviewer -a Rudi openSUSE:Factory
When reviewers in the project and inside the package are defined all of them gets added as reviewers to a new request.
As an example openSUSE:Factory may get the following reviewer groups in the meta data according to our current policies:
<group groupid="legal-team" role="reviewer" /> <group groupid="source-review-team" role="reviewer" />
And openSUSE:11.3:Update may get:
<group groupid="security-team" role="reviewer" /> <group groupid="legal-team" role="reviewer" /> <group groupid="source-review-team" role="reviewer" />
Request review on selected requests
Any target owner may want to rask another person or group for additional review if a particular change is questionable. So for example, when there is a submit request which adds a setuid root bit you may want to ask the security team first. You can do so easily in the webui or with osc:
osc review add -G security-team $REQUEST_ID
Process reviews
The webui of OBS 2.1 has functionality to list, accept and decline reviews. It also lists open reviews in your "my requests" list. osc 0.129 supports listing of reviews:
osc review list
or accept/declineing reviews:
osc review accept/decline $REQUEST_ID
Technical Implementation Details
This draft describes the enhancements to the request system to request and perform a review.
The review is an optional state which can be set by the request creator or by the request target owner. It is intended to be used, when one or more parties shall approve a change before it get accepted. A request may contain multiple reviews.
Examples when reviews make sense are, when another package is affected and the packager owner should get aware of it. Also reviews by teams like security, maintenance or QA teams are often needed.
The reviewer may suggest an alternative solution, which he needs to prepare in an own project/package.
The request should get proceeded after all reviewers have accepted the state.
Therefore a request xml can contain one or more of the following snippets:
<review state="superseded" by_group="maintenance" who="Harald" when="20090810"> <comment>looks good, but minor changes are needed</comment> </review> <review state="new" by_user="adrianSuSE" />
This example describes a processed review. The "maintenance" group was requested to process the review. Harald did it on 200900810 and accepted the submission, but has fixed something, so he suggest to use his submission instead. The alternative submission is specified in the request state element by the "superseded_by" attribute".
Another review shall be done by the adrianSuSE user, this review is not yet done. This will not get done in this request anymore, since the state of the request has been set to superseded and a new request got created.
- state can contain "new", "accepted", "declined" or "superseded".
- user or group can be given, never both.
- superseded_by is required on superseded state.
Add reviews or state changes of a review item is done via a request command:
POST /request/:id cmd=addreview&by_group="security" POST /request/:id cmd=changereviewstate&by_user="adrianSuSE"&newstate="accepted"&comment="Free text comment"