Github and Code Reviews

We all like to do code reviews, but in practice code reviews don't happen as meticulously as intended. I know of two reasons:

1. Code reviews create extra work. Print outs, folks sitting in meeting rooms in front of laptops, or submitting diffs to a tool are all extra work for the team.
2. Code reviews are out of band. Though some tools are integrated with source control systems, they are still not part of the commit process. Consequently, they get ignored over time.

But since we moved our project to github, we have made code reviews a part of the flow without adding extra out-of-band work to anyone in the team.

Before we started, code reviews were not our top priority. We had the following goals in stead:

1. Keep the head always stable, tested, and ready for deployment.
2. Keep the other members in the team abreast of code changes happening. This is particularly needed by newer team members.
3. Ensure code quality.
4. KISS and DRY.

In order to make these happen, we adopted the following model.

1. Create repos under an org.

First create an org in github, and then create a team with with push and pull rights, and add members to the team.

2. Use forks for development.

Every member forks the repo, clones the fork, and works against the fork. All code pushes happen in the fork.

3. Avoid cloning the main repo except for merging purposes.

Since forks are used for development, there is no need to fork the main repo. To avoid any confusion, if cloning the main repo is required, clone in a temp directory and nuke it after the work is done.

4. If you make an accidental commit to the master, apologize to the team, and back off your changes.

This happened early on a couple of times, and we decided that the best course is to back off those changes as soon as possible.

5. Submit pull requests when ready.

Once the code is pushed to the fork, submit a pull request.

6. Let someone else review and merge pull requests

This is when code reviews happen. Github sends out notifications to the team members when a pull request is made, and someone else in the team reviews and merges the code. Any discussions and comments happen on the pull request page which leave a trace of all discussions for later reference.

Code reviews are an integral part of this flow. We take some additional steps to make code reviews easy and effective.

1. Keep commits and pull requests small. In particular, avoid submitting pull requests that do fix/change more than one thing.
2. Include tests in each pull request.
3. In most cases, reject a pull request if there is no test included in it.

Large commits usually get less scrutiny due to the mental comprehension load, and the above steps help us pay enough attention to each change.

Thanks to Github!