

Practical Lessons in Peer Code Review - mgertner
http://blog.salsitasoft.com/practical-lessons-in-peer-code-review/

======
tcopeland
> 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.

------
comatose_kid
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.

~~~
xyzzy_plugh
This is very much a cultural thing. I don't necessarily agree that you get
better feedback or better understanding through an in-person meeting.

In my experience tools like gerrit allow developers to learn and review at
their own pace, and provide well-articulated, thought-out feedback.

Note also that a large amount of modern development still occurs on mailing
lists.

It's all about culture.

------
lberger
"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.

