How to monitor code review efficiently?

Question

I suspect major code review cover up in my team. Too many code reviews are merged without any comment.

Seems to me like there's no such thing as a code review without a single comment.

How can I as a team lead properly monitor that my team is doing a proper code review process and how can I help them to maximize the process' benefits?

Update

Thought people might want to know about any update. I tried a lot of suggestions that were given here. most were already in use. some helped a bit. However, the problem remained - some people continuously got bad code in when I was not looking.

I found that code review monitoring is not as helpful as giving my team tools to make their code better to begin with.

So I added a library named "jscpd" to detect copy pastes. The build failed on copy pastes. That eliminated one problem immediately.

next we are going to try codeclimate.

I am also doing a manual review on old code reviews once a sprint for half a day. I am converting todos into issues/tickets - as I found out people are writing them, but they are never handled at a later point. I am also doing meetings with the entire teams to review code when it is appropriate.

in general it feels like we are moving in the right direction.

Answer 1

I'm going to offer a different take from my fellow answerers. They are right - be involved if you want to see how things go. If you want more tracability, there are tools for that.

But in my experience, I suspect that there's something else going on.

Have you considered that your team may feel that the process is broken/stupid/ineffective for most commits? Remember, process is documenting what works well, not rules to obey. And as the team lead, you're there to help them be their best, not enforce rules.

So in your retrospectives (if agile) or one on ones (if you're a manager) or in random impromptu hallway meetings (if you're a non-agile team lead and there's another manager doing one on ones), bring it up. Ask what people think of the code review process. How is it working? How is it not? Say you think it's maybe not benefiting the team as much as it could. Make sure you listen.

You can do some advocacy for code reviews in these meetings, but it's better to listen to the feedback. Most likely, you'll find that either you team thinks that the "proper" process needs adjusting, or that there is some root cause (time pressure, lack of reviewers, Bob just commits his code so why can't we) to address.

Forcing a tool on top of a broken process won't make the process any better.

Answer 2

I dislike posting one-line answers, but this one seems appropriate:

Participate in the process.

Answer 3

Get a tool, like ReviewBoard or Redmine's codereview plugin. Then each review is created as a task that has to be closed or commented upon by someone (just like a bug ticket). Then you have traceability of who created the review ticket, and who closed it. You can tie review tickets with source code checkins, ie create the ticket from a revision.

Answer 4

You could document what the team wants in code reviews that you've discussed and agreed with developers. Some things you could consider as part of code reviews are:

• Check that the code does what it's supposed to do i.e. it meets the requirements

• Code style to ensure that developers are coding to a consistent style

• Optimisation e.g. number of function calls

• Architecture and reusability

• Exception handling and logging

• Technical debt: is the code in a better state than when the developer started working on it

• Check out and build the code (I find this useful but other devs in my team prefer to leave this to testers)

• Using an automated tool (I've used SonarQube). I find it useful to integrate this into your build process to enforce improvements to the code e.g. increasing the test coverage

Some of the steps above can be covered by an automated tool but while you're trying to improve the way code reviews or done it's probably worth using both the tool and eyeball review. However, the most important steps for preventing technical debt (architecture and reusability) cannot be wholly automated.

If your team is inconsistent in applying this you could try only allowing the developers who are carrying out code reviews properly to have merge rights. For example, you might just want to start with the lead dev on the team. The trade-off with this approach is that those developers could become a bottleneck in the development process, so you and the team need to decide if you want this. Personally I would accept this trade-off and through the code reviews increase the discipline across the rest of the team and then when ready you can increase the number of developers with merge rights.

Finally, it's worth reviewing the reviews. So once a week get together with the developers and constructively discuss the reviews and ways of improving them.

Answer 5

A few things (to be honest, most of these are covered across the answers, but I wanted to put them in a single place)

• You can put process and rules in place to make sure a code-review happens, but it's pretty impossible to put them in so that code-review is actually more than a box-ticking exercise. Ultimately the team has to see the benefit of the process, if they are to approach it usefully

• Lead by example. Take part in reviews. As a developer, I feel bad if my manager (a non-developer now) spots stuff I don't. Highlight issues that should have been caught in review (in a non-blaming way). If a production issue happens, if issues arise during QA (if you have a separate QA process), highlight where they could have been caught in code-review. Discuss with the team how can we can ensure future issues like that are caught

• Discuss with the team what they want the process to do. If they don't see any point to it (as may happen at the beginning) use the production issues and necessary refixes as evidence of its benefit

• Use automated code-checking software like Sonarqube so that code-reviews can focus on issues like incomprehensible-code, logic errors, lack of documentation, etc that can't be spotted automatically.

Answer 6

I'll tell you how my team quickly integrated code review into its workflow.

First, let me ask you a question. Are you using a version control system (e.g. Mercurial, Git)?

If your answer is yes, then proceed.

1. prohibit everybody from pushing anything (even small fixes) directly to the master branch (trunk)*
2. develop new features (or fixes) in separate branches
3. when developers believe the branch is ready to be integrated in master, they will create a "pull request"
4. prohibit everybody from merging their own pull request*
5. have another developer evaluate the pull request and review new code
6. if the code passes the review, good, the pull request can be merged, otherwise fixes can be applied
7. repeat step 6 until code matures enough (can be done without starting over)**
8. done, all your new code gets reviewed (at least summarily), by someone with a name

Now you have a precise point in your workflow where code review gets done.

Act there.

* can be enforced automatically, with server-side hooks

** this procedure is fully supported by GitHub (among others), and is fairly easy to use, check it out

Answer 7

I think you should create a template and ask your team members to update it every time they do a code review. But even then, you should participate in the review process initially.