Where I work, we are eight software developers and every task done must be reviewed by another developer. Often, I feel like the code reviews are done too quickly (example: a task completed in five hours gets reviewed in 15 minutes). I know I can easily spend an hour reviewing a task that took five hours to complete.

Here is what we agreed, as a team, to verify when we do a code review:

  • The code written/changed solves the problem (bug fix, new feature, etc.)
  • Code is clean and well structure (to our standards)
  • Code style is correct (to our standards)
  • Documentation, if any, is up to date
  • Unit tests cover all the possible cases and do not fail

I was wondering if it's my code reviews that are too long or if it's the code reviews of the others that are too short. Or maybe both?

Edit 2015-10-06 15:55 EST : I agree that there is no one-size fits all answer to the question as mentioned by Thomas Owens♦. I guess I was more looking to what should trigger a flag for a code review too short or too long. Or should I simply don't care and let the code fail in production because someone botched a code review (because it does happen)? Remember, we did, as a team, agree to verify that the code solves the problem. We have no QA team.

I ask because it happens that code pushed into production fail. While I agree it can happen, it happens too often in our team. When we talk with the developers to learn what happened, we learn that the code review simply didn't happen (this is more a process problem than a code review problem) or the code review was deficient / not tested enough.

  • There is no correct length for a code review. But in general code should be tested and analyzed proportionally to how important the code is. – Canadian Coder Oct 6 '15 at 18:44
  • 2
    Half of your points can be largely checked by linters/static analysis/CI servers/code coverage analyzers, and do not require human interaction. Usually, the point of code reviews is knowledge transfer, not babysitting each other. Why is your organization doing reviews? – amon Oct 6 '15 at 19:01
  • @amon: Mainly to detect bugs. We do not have continuous integration tools (we're working on this in our "free" time). We check code style because some developers do not use the automated code formatter inside the IDE (I know they should!). – user92912 Oct 6 '15 at 19:10
  • @SimonArsenault They don't need to use the code formatter in the IDE. You can write hooks in your version control system. – Thomas Owens Oct 6 '15 at 19:13
  • 3
    Are you sure that this is a code review problem? Do your developers have sufficient information about the problem and/or access to people who do? Do you have unit tests? Do you developers run tests (automated and/or manual) before pushing changes? More code review is likely not the answer. Seems more like an X-Y problem with the extra details. – Thomas Owens Oct 6 '15 at 20:01

There's no one-size fits all answer to this question. The amount of effort for a peer review will change depending on the objectives of the review, the complexity of the change, and the level of formality. There is some guidance that you can apply.

The first thing to do is to understand why you are doing code reviews. Specifically, I'd argue that ensuring that the code solves the problem isn't part of a code review. For a defect, the person fixing it should have enough information to reproduce the issue. With sufficient acceptance criteria and test cases, they can not only ensure that the fix is appropriate, but also perform some level of regression testing. I'd also argue that ensuring compliance to code style and standards is something that large portions of can be automated.

The biggest thing that I would recommend is a divide-and-conquer approach.

Prior to code review, use automated tools as much as possible for analysis and style checking. Use static analysis to find issues before it is submitted to humans. Use tools to reformat the code per your team's style guidelines. Try to minimize the work that needs to be done by human reviewers.

Once you are focusing your human reviewers on things that tools can't catch, consider having more than one person participate in the review and divide the work up into two blocks. It's easier to get someone to spend an hour than it is to get them to spent 2 hours, so maybe have 2 people each spend an hour looking at different things. Depending on your team organization, there may be different ways to divide up the review.