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?

Qiulang 邱朗
  • 3,095
  • 3
  • 13
  • 22
  • 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 '22 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 '22 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 '22 at 14:35
  • 1
    Why these down-vote and closed vote? Is there anything I missed here ? – Qiulang 邱朗 Nov 25 '22 at 15:57
  • @Qiulang邱朗 software developers don't like when you say software developers are doing something stupid :) – user253751 Nov 25 '22 at 16:57
  • 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 '22 at 19:55
  • @DocBrown I didn't answer that because when I see the words "what is the purpose of your CRs" I don't know what to say. And why is my way is "bit of a red flag here" ... So I don't answer. – Qiulang 邱朗 Nov 26 '22 at 03:18
  • @DocBrown I did not expect strangers from the internet to give you a better reply but on the other hand I would be surprised if people tell me that no that never happen in my team, I don't have this problem. – Qiulang 邱朗 Nov 26 '22 at 03:25
  • 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. – Ben Cottrell Nov 26 '22 at 11:07
  • @BenCottrell How many time you know something is important but you didn't do it? For starters, I know a healthy diet is important but I still indulge myself in junk food. – Qiulang 邱朗 Nov 27 '22 at 01:40
  • @Qiulang邱朗 That sounds like you're assuming that the developers themselves are to blame rather than it being a problem elsewhere. The point of my comment is to not make such an assumption, but instead talk and assert the real reason, as it's likely the root cause may be the review technique itself. Peer review should be a chance to share knowledge and keep the team on the same page, so if that's not happening, and it isn't helping them collaborate then you need to talk to them and learn why -- it may be that their approach/technique around peer review itself is misguided. – Ben Cottrell Nov 28 '22 at 08:08
  • @BenCottrell I didn't assume anything, I just gave you a counter argument. I have searched the site for the similar question, apparently it is not just me who has this problem. I will update my question later. – Qiulang 邱朗 Nov 28 '22 at 10:49
  • @DocBrown please see my updated question and https://softwareengineering.stackexchange.com/questions/39855/how-to-deal-with-someone-who-dislikes-the-idea-of-code-reviews, apparently there are some good answers there (from some total strangers) – Qiulang 邱朗 Nov 28 '22 at 12:24
  • @Qiulang邱朗: oh, I did not say you cannot get any answers here. I am saying you should first talk with your team, you will probably get much *better* answers from them (and you question here should include what your team mates told you). – Doc Brown Nov 29 '22 at 15:23
  • @DocBrown well the simple answer is they are sometime sloppy, they are inexperience; they feel they have more important things to do(that is the case some times). And that is why I said in my question I don't want to get into the root cause here because that will sidetrack my question. Here I just want to know what method(s) people ever tried to deal with that. – Qiulang 邱朗 Nov 30 '22 at 02:54

5 Answers5

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.

Bart van Ingen Schenau
  • 71,712
  • 20
  • 110
  • 179
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.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
  • Your answers is inline with what https://engineering.fb.com/2022/11/16/culture/meta-code-review-time-improving/ said. BTW, there was probably a typo here " the most common problem is the time it takes the have the review." – Qiulang 邱朗 Nov 26 '22 at 04:18
  • @Qiulang邱朗 better now? – candied_orange Nov 26 '22 at 10:55
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.

Jon Raynor
  • 10,905
  • 29
  • 47
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.

Computable
  • 121
  • 3
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

Cap Barracudas
  • 1,001
  • 9
  • 17