Test Review Process Revision

[fantasai]

HP has been test-driving a wiki-based test submission system, and
one thing that came out of that discussion is that the review process
seems to need some tweaking. Melinda and I have been considering the
following proposal.

The current process is

   1. Contributor submits a test.
      -> Submitted
   2. Someone who doesn't work for the same company OR an
      owner or peer reviews it, and if it passes review,
      approves it.
      -> Approved
   3. Someone (usually an owner or peer) checks it in.
      -> CheckedIn

The proposal is

   1. Contributor submits test.
      -> Submitted
   2. Anyone reviews and, if it passes review, accepts it.
      -> Accepted
   3. Owner or Peer either accepts reviewer's judgement (if
      known to be competent) or reviews the test himself.
      -> Approved
   4. Someone (usually the owner/peer in #3) checks the test in.
      -> CheckedIn
where steps #2 and #3 can be collapsed if an Owner/Peer is reviewing.

The change has two effects:
   a) It requires all tests to pass review by someone known to be
      competent rather than just anyone. (However this person can
      still be someone other than an Owner or Peer.)
   b) It encourages tests to be pre-screened by other contributors
      (who could be employees of the contributor's company) before
      hitting the bottleneck (Owner/Peers). This will hopefully also
      have the side effect of training more competent reviewers.

Some technicalities:
   - The checkin comment should name the peer reviewer: r=name
     if s/he reviewed the test, rs=name if s/he relied on the
     previous reviewers' judgement.
   - For CSS2.1 we have a "Junior Peer": under this process he
     would qualify to perform step #3 subject to the current
     same-employer restrictions.

Comments?

~fantasai

[fantasai]

fantasai wrote:
>
> HP has been test-driving a wiki-based test submission system, and
> one thing that came out of that discussion is that the review process
> seems to need some tweaking. Melinda and I have been considering the
> following proposal.

Ok, I've updated the review guidelines in line with that proposal.

   http://csswg.inkedblade.net/test/css2.1/review

~fantasai

[fantasai]

Melinda asked for another change in the review process to make it more efficient,
specifically, allowing the reviewer to directly make the changes necessary for
the test to pass review. I've updated the guidelines to say the following:

   If the test does not pass review, the reviewer must tell the submitter what
   is wrong with the test and what steps should be taken to correct any problems.
   Typically the submitter is responsible for fixing the tests, but if the reviewer
   has permission (or the tests have been abandoned) s/he may make the changes
   directly and then explain to the submitter what they were and why they were
   necessary. In this case the submitter (or, in the case of abandoned tests,
   another reviewer) needs to review and accept these changes.

This keeps the requirement that all changes made by one person need to be reviewed
by another person, in this cases the reviewer and the submitter switch roles. It
also makes sure that the submitter understands what the problems were so s/he can
learn to make better tests the next time around.

~fantasai