1

I am dev manager, managing 8 developers. I am facing a problem that the code review becomes more and more a formality even though we all know the code review is important. We do have code review process but when people don't want to do it they can find the way to circumvent it.

I hate to be bossy around, telling my guys "please let's review this together or please take it seriously" all the time. So what are some specific procedures I can take to let them do code review more seriously? Currently I only have two: (a) assign a different guy to fix the bug caused by someone else's code. To fix it he/she has to read and understand others' code first. (b) do code review with them from time to time. But doing that has its own problem too.

I suspect people may suggest me to look into the root cause, find out why they don't want to do it wholeheartedly and fix that instead. But for this specific question I want to focus on what methods I can use (because discussing the root cause may sidetrack my question)

A comment said "The fact you expect strangers from the internet to give you a better reply than your team...". That is NOT what I expect but I guess it said a doubt people may have so I am trying to clarify that: I would be surprised if people tell me that "NO, that never happens in my team, I don't have this problem, or my guys love to do code review". So I was asking some general advices you may have tried to deal with that.

Another comments said "If they knew code review was important they'd be doing it." My counter-argument is even simpler, I know a healthy diet is important but I still indulge myself in junk food! I don't want to delve in the "root causes", which can be another question. Here I simply ask what methods have you ever tried, and if that evolves the root cause in your case I am all ears but if you say "I don't have this problem, my guys just love code review" then this question is not for you.

--- update ----

I search the SE site and find at least two questions were somewhat related to mine,

  1. How to deal with someone who dislikes the idea of code reviews?

  2. How to monitor code review efficiently?

15
  • 2
    Did you ask the team members themselves? They may have better suggestions than anyone who does not know your team.
    – Doc Brown
    Nov 25, 2022 at 14:13
  • 4
    really need some more info here. what do you cover in your CRs? what is the purpose of your CRs? Why cant you just mandate them in your CI pipeline for all merges? are they being skipped completely or just ticked off without achieving your purpose?
    – Ewan
    Nov 25, 2022 at 14:32
  • 1
    "assign a different guy to fix the bug caused by someone else's code" bit of a red flag here tbh. how about you just put bugs in the backlog and not worry about who's fault they are?
    – Ewan
    Nov 25, 2022 at 14:35
  • 2
    @Qiulang邱朗: I did not downvote or close vote (yet), but you could start to answer Ewan's questions to improve the question. And, you are asking us something which you really should discuss with your team, as I wrote. The fact you expect strangers from the internet to give you a better reply than your team gives me the impression this could be more more a workplace issue than the software engineering issue.
    – Doc Brown
    Nov 25, 2022 at 19:55
  • 2
    @Qiulang邱朗 You mention "We all know code review is important", but the rest of the question implies the team believes exactly the opposite. If they knew code review was important they'd be doing it. The fact it's seen as a formality almost certainly means they think it's a waste of time. What's missing is why they don't think it's important, yet the only way to understand this is talking, honestly and openly; at this point it's primarily an interpersonal/communication issue, requiring trust from everyone involved. Nov 26, 2022 at 11:07

5 Answers 5

9

When people start treating part of the process as a formality that just needs to be ticked off, without paying any real attention, then the only way to resolve that situation is to find the root-cause why that part of the process is not considered important enough to pay attention to. Then you can address that root-cause and fix the perception issue.

As long as the developers don´t consider that step in the process as important enough, they will find a way to work around any method you come up with to make them pay attention.

4

In a healthy coding shop the big formal peer review at the conference table is redundant if not outdated.

The most important thing you can do to make a peer review effective is to do it soon. While I still have the code in my head. Wait until I'm thinking about some other ticket and I'll be very reluctant to make any changes. Faced with that peer reviewers loose motivation because getting anything changed is such a fight.

The big formal peer review with the whole coding team at the conference table picking over a code print out is the worst offender in this respect. It takes so long to get everyone together that it practically ensures the author has moved on by the time it happens.

An improvement on that is use of tools (source control, email, chat, etc) to review code as soon as it's committed. This can minimize the time it takes to review.

Pair programing pushes the review time down to zero because a peer is watching me type. This gives me feedback soonest, when it's most welcome.

Many things can make a peer review go wrong but the most common problem is the time it takes the have the review. This causes the most pain.

Another problem is when these more advanced ways of reviewing aren't respected by the formal process and more credit is given to the slowest forms of review. In that system good coders will treat the big formal review as a redundant pointless ceremony to be mindlessly ticked off because all this work has already been done.

And it is entirely possible to demotivate people from caring about peer reviews because the more fuss you make over my code the more I retaliate when I review your code. This can happen because the schedule put too much time pressure on people. That combines nastily with slow reviews.

The fix is to give people the freedom to review quickly, apply review feedback quickly, and recognize that work, even when it's just two people talking over the same keyboard.

When someone claims their code has been reviewed don't demand a ceremony, enforce use of a tool, or formality. Just get the reviewers name.

Also, understand that even in the healthiest of shops bugs will still sneak through. Don't assume something is wrong with the reviewing process just because it's not airtight. It’s never going to be.

2
2

They probably don't want to do it because it cuts into their own time doing their own work. Ideally, peer reviews are for peers. But it sounds like this isn't happening. In your case, if you have a team of 8, I would divide it into team sub teams of 4 with one person being the lead, so you have 2 leads. The lead is responsible for doing the code reviews, optionally each person (peer) should be reviewing as well. One can put in a rule that 1 tech lead must approve in order for the code to be merged. The tech lead can budget X% of their time for code reviews. This should solve the problem of reviews being done, although tech leads may not like it.

Most of the code review process should be automated. Linting, test coverage, code complexity, etc. should be automated and if the code isn't meeting expectations, then one can't check it in. Don't waste people's time with something that can and should be automated. This should cover 80% of the review process without a human looking at it. The tech lead should look at the code and make sure the person isn't doing something off the rails. Sometimes, it just pointing the developer into certain tribal knowledge about the code base or looking for performance improvements, etc. Line by line assessment is usually not needed. It's usually at a higher level looking at the implementation/solution holistically. Syntax and other things should be managed by automation.

Also, I would post the reason(s) why you are doing code reviews. It should be obvious (shared responsibility) but maybe having it written down would help. I always enjoy doing code reviews because sometimes I learn something new since people write code in many different ways. So, it can be a learning exercise as well.

1

I would approach it from a failure analysis/metric perspective.

Monitor and analyze software failures/bug reports and evaluate where such a bug/failure could have been discovered at its earliest point: requirements complexity/review; design guidelines/reviews; code reviews; unit testing; subsystem testing...

From this analysis it may become clear whether the code review is of value.

1

I see this behavior quite often. I would say your best bet it to lead and persuade them by explaining why these are all important.

They might not want to do it anyhow but at the end of the day that is the job. Not only producing working code but also managing risk, on boarding time, code quality, knowledge transfer and other things which are all affected by the code review process.

Have a look at this answer too for some more arguments:

https://workplace.stackexchange.com/a/80325/111711

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.