Share via


Code reviews on the test team

I've mentioned a few times in the past that before we (the test team) check in any code we have to get it reviewed. I don't think I ever mentioned the process before so here is what we do. This will pretty much follow industry standard guidelines and practices so there won't be anything outlandish or groundbreaking here, but I did want to give everyone an idea of our expectations.

First, check out and make the code changes you need.

Next, we package the changes into a .DPK file. We have command line utilities to do this, or mostly we use the code review tool - an in-house created tool - to grab the changes and upload them to a common server. This same tool also provides a windiff type of display so we can see the changes. This tool also allows comments to be made so we can write up notes on the code. Oddly, we don’t use OneNote for this…

Anyway, an email will go out to the entire test team that there are changes to be reviewed. Or, you can send the mail to specific stakeholders if that is appropriate. Anyone that wants to can jump in and review the changes, and although we have a requirement that at least one other person reviews the changes, there is no requirement that any one person has to review this particular change. This may vary a bit - for instance, the key ink testers review ink automation, the storage folks look at those tests, and so on. Here we check for design, implementation, comments, clarity and coding guidelines - again, pretty standard items. If we are satisfied, we can mark the review complete. If there are non-trivial changes, the author has to make the changes (or defend her code and implementation) and either submit a new review for another round or get agreement from the dissenting tester that the code is sufficient as is.

The final key here is that you have to get buyoff from everyone that participated before you can check in. Even if I say that the changes are sufficient, if Alice does not agree, you have to work with Alice to reach agreement.

Then you can check in your changes and move on. In general, this should not take more than a day or so to finish end to end.

Questions, comments, concerns and criticisms always welcome,

John