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: assigns the issue to
apireviews@netbeans.org. The submitter
can also add
API_REVIEW_FAST keyword if he believes that this
architecture change is
trivial, non-controversial and does not cause any
compatibility problems.
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.
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, and such changes must have no negative
impact on existing modules.
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.
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 commiting the solution to CVS 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.
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.
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.
Comments to:
apireviews@netbeans.org
