APIReviewSteps
From APIDesign
JaroslavTulach (Talk | contribs)
(New page: Copy of netbeans:APIReviewSteps created for archival purposes. ==Architecture Review Steps== Here is a step by step description of API review process. It describes two types of API r...)
Next diff →
Current revision
Copy of netbeans:APIReviewSteps created for archival purposes.
Contents |
Architecture Review Steps
Here is a step by step description of API review process. It describes two types of API review: the fast-track review and the standard review.
These are the roles that different people play in reviews:
Submitter: the person proposing API change and asking for review
Submitter's task is to prepare information for review, properly announce it, drive the review procedure, answer questions during review via mail/issuezilla and in the face to face review, and record the result of the review
Reviewers: people assigned or choosen by the reviewer to do the review
Reviewers need to study the issue and provide feedback and final decision in a timely fashion, they need to participate in face to face meetings for the reviews they are responsible to verify that the result of the document is correctly recorded in the summary document and/or issuezilla.
Reviewers Chair: lead of reviewers group
Not needed. At least not needed if things go well. In case of a dispute, chair is the one to find and decide on acceptable resolution.
Terminology:
- TCR - Technical Change Required
- TCA - Technical Change Advised
Asking for a Review
Before a review can be requested the submiter has to prepare documentation that is supposed to be reviewed. Then the type of the review has to be selected.
Step 1 - Prepare Materials
Submitter: Create an issuezilla issue asking for review with links to materials for review (best is to setup easily editable wiki page). The issue should include the following information:
- A short issue description outlining what the issue is and why it is being done.
- Target milestone
- Dependencies on other issues and issues that depend on this one
The materials or the issue also need to provide additional information:
- An explanation of the change in architecture or API.
- A list of the interfaces impacted by the change that the module offers (imports) and depends on (exports).
- The specification (e.g. javadoc) and stability category (aka commitment level) for each interface.
- If there is an existing document with answers for Architecture Questions and the issue makes only a partial change to the architecture the architecture document needs to be updated to cover the proposed change in order to qualify for the fast-track review.
Step 2 - Submit the Issue
Submitter: Add apireviews@netbeans.org on CC or as an assignee of the issue. Add API_REVIEW or API_REVIEW_FAST (if this architecture change is trivial, non-controversial and does not cause any compatibility problems) keyword.
Step 3 - Decide the Type of Review
- if the issue has been annotated with API_REVIEW_FAST keyword when submited, then it can either be accepted (by doing nothing) or any reviewer (any developer with access to issuezilla) can change the type to API_REVIEW. Such change shall be made during two working days after submition of the issue.
- if the decision is to choose API_REVIEW_FAST the next task is Fast-Track Review, Step 4.
- when a standard review is selected, the process continues with Standard Review, Step 4.
Fast-Track Review
The fast-track review is an email/issuezilla-based review technique suitable for trivial changes to already reviewed modules/APIs. The two litmus tests to use when choosing fast-track:
- The change must be simple.
- The architectural impact of the change must be obvious and non-controversial.
Fast-track type changes generally apply common practices in frequently performed tasks like additions of individual methods into already published API, and such changes must have no negative impact on existing modules. Please bear in mind, that the term API has wider meaning and includes anything observable from outside. For example, adding a new dependency, or restriction on the execution environment of the API may deserve a deeper evaluation than a method addition.
Fast-Track Step 4 - Review
Reviewers: Every reviewer is expected to review the issue. They can ask questions, discuss the issue with submitter or within reviewers group if needed. As a result of their review, they shall:
- fill TCRs as DEFECT issues blocking the reviewed issue (add TCR into status whiteboard). These issues will have to be resolved before implementation is commited into CVS.
- add TCAs either as comments into the issue or as separate issues blocking the original one (add TCA into status whiteboard). These advices need not be implemented when commiting into CVS.
- request standard review, if the issue gets more complicated than originally expected. Add a comment to the issue and remove API_REVIEW_FAST keyword (a reviewer can suggest need for standard review by leaving just API_REVIEW).
Submitter: wait for the review period, if there are questions resolve them within that period.
Fast-Track Step 5 - The Decision
Summiter: If nobody objected against this issue being fast track (by removing API_REVIEW_FAST for a week since submiting, the summiter shall reassign the issue back to himself and put apireviews@netbeans.org on CC. He should also add a comment describing what he is going to implement (TCRs are must, but he can select just some TCAs). Then he should wait 24 hours before pushing the solution to default branch to give reviewers last chance to comment. After that, he can close the issue.
Chair: If any reviewer removed API_REVIEW_FAST keyword the chair has to step in and decide what to do. He can reject the issue completely by closing it as WONTFIX or turn it into standard review
There are 4 possible decisions:
- Accepted
(No blocking issues, Go to implement or commit, based on phase of review) - Accepted with change requests
(No major TCR blocking issues, Go to implement or commit with completed TCRs) - Rejected
(TCR blocking issues filed, Go back and do it again.) - Needs standard review
(During the review the issue has been found as too complicated or
controversial. Submitter needs to complete the information for standard review and goto Standard Review, Step 4)
Standard Review
The standard review process requires the submitter to fill in the Architecture Questions document. The issue needs to be reviewed once in the inception phase and then before commit. The submitter needs to find four reviewers (from the people who know the domain, for users of proposed API and from those who understand how to design an API) for the case and towards the end of the review period there is a face to face meeting where the final decision is made. The decision is summarized in the Opinion document or in the "easily editable wiki page" as prepared when submitting the issue. The details description of steps follows.
Standard Review Step 4 - Prepare Materials for Standard Review
Submitter: finishes the set of answers for Architecture Questions based on if the review is an inception review or before commit (both the second and third set of questions needs to be completed for the review before commit). Submitter puts links to appropriate documents into the issue or the wiki page and assigns the issue the apireviews@netbeans.org.
Standard Review Step 5 - Assign Issue
Submitter: Adds four selected reviewers on CC of the issue and mentions them in a comment. Negotiates a time for review data and puts it into the Status Whiteboard, or as a comment into the issue.
Standard Review Step 6 - Review the Materials
Reviewers: review the issue, ask questions, discuss the issue with submitter or within reviewers group if needed. All technical change requests must be filed as DEFECT issues that block the reviewed issue. Issues with TCR in status whiteboard must be resolved otherwise the issue is rejected (those are usually reproted as P1 or P2 DEFECTs). Issues with TCA in status whiteboard (usually DEFECTs of lower priority) do not block accepting the issue but must be resolved as part of implementation. Assigned reviewer schedules time/place for actual review before or on the due date.
If the review cannot be completed by the date in status whiteboard this has to be clearly communicated to the submitter.
If the reviewers find that the documents provided by submitter are missing important information they can assign the issue back to submitter. They will mark the issue by adding "NeedSpec" into Status Whiteboard. The submitter will add the information, remove "NeedSpec" from Status Whiteboard and reassign back to the responsible reviewer.
Submitter: if there are questions resolve them within that period
Standard Review Step 7 - Hold Review Meeting
Reviewers: meet to review all the opinions and issues in this meeting and make the final decision (by voting).
If the reviewers find that the documents provided by submitter are missing important information they can assign the issue back to submitter. They will mark the issue by adding "NeedSpec" into Status Whiteboard. The submitter will add the information, remove "NeedSpec" from Status Whiteboard and reassign back to the responsible reviewer. Reviewers can either hold a new review meeting or review the documents and discus them off-line.
Submitter: participates in the meeting to answer questions.
Standard Review Step 8 - Write the Opinions Document
Submitter: Write the result of the review into the Opinion document (or the wiki page). Proceed with work on the issue, based on the decision. If the issue was reviewed in the inception phase the submitter needs to come back for the review before commit using the same process. If this was the review before commit the submitter can integrate the change. The decision is binding for the submitter (i.e. no commit into trunk should be done before the review finishes in Accepted (with TCAs) state).
Reviewers: Make sure all their feedback is in the Opinion document. Make sure the document and the issue are crosslinked.
Follow
