Practical Lessons in Peer Code Review

> If you are including third-party code in your patch, > commit it separately.

Sometimes I'll do that with changes that involve new service client libraries. First commit will have the new library fooservice_client along with its config files and such. Once that gets deployed I can open up a production console and verify that the app can query the service and display the results. Then a followup commit will contain the actual change that I want to make - "query foo service for a list of bars and display them" - and I'll put a note about how the connection credentials and setup have already been verified.

Of course that depends on an environment with more or less continuous deployment. If deploys are two weeks apart this approach is just going to take to long to get anything done.

My recommendations when checking in a new feature or substantial change:

1. Meet in person

2. The coder should print out copies of the diffed code for each attendee

3. Have roles in the meeting - someone to read the code, someone to capture bugs + severity, and the rest are there to review the code

4. Coder takes documented defects, fixes them and reports back to the group with diffs.

5. Metrics can be kept, used as feedback to improve process.

Usually 4-5 people are sufficient for a large chunk of functionality.

Is this process slower than gerrit? yes. Do you get better feedback, and better understanding (at a team level) of the code base? yes.

I haven't seen software that really emulates this experience - there is an interesting opportunity here.

"It is instructive to consider pair programming, which is a fairly popular practice and adds an overhead of 100% to all development time, as the baseline for code review effort."

I couldn't disagree with this line more. I've frequently seen it halve the amount of development time.