Divmod Best Practices: Code Review

Divmod uses code review for just about every change to any project.

Code review is a pretty simple process: at its heart, it is little more than reading some code written by someone else. Nevertheless, it can be useful to have a set of things on which to focus during a review:

• Does the branch merge or the diff apply cleanly?
• Are there unit tests for the code being changed or added?
• Do the unit tests pass for you?
• Do the unit tests pass for buildbot?
• Is there documentation for new code?
• Where appropriate, has existing documentation been updated (including ChangeLog/NEWS files)?
• Does the code adhere to the coding standard?

There's the easy list, most being mechanical checks. For a more thorough break-down, please read the Twisted Review Process.

Don't feel bad about rejecting a branch if the answer to any of the bulleted questions is "no"; the problems may seem minor in isolation, but each contributes to overall poor code quality. Moreover, sometimes apparently minor problems can be hiding larger issues.

Taking a look at the bigger picture can be a little bit harder, but it's just as important. As with most important skills, it takes time to gain proficiency in this, but everyone benefits when you do.