How can I get my coworkers to review my code?

Question

For context: my productivity as a software developer is tied to delivering my work, and delivering my work is tied to having my code reviewed and approved by co-workers.

But lately I've been having trouble getting co-workers to start the code review. For easy ones (one-line changes) it would takes a few hours, which is fine, but I've had critical tasks that would block others taking multiple weeks. It's never too large or particularly hard to review – anything more than 300 lines would be broken down.

This is the second time this has happened in my career.

At a previous company, I had a manager would have to intervene from time to time. Once he rage-merged all my Pull Requests that were open for more than two weeks because the rest of the team was not reviewing my work.

I routinely make an effort to prioritize reviewing other people's code and I'm always the top reviewer in my team according to our reports... both in time and in amount of reviews/amount of lines of code reviewed. When it comes to juniors I try to make an effort not to be too hard or too serious, I always give positive feedback as much as negative, and I always try not to leave too many comments...

In both teams I've been among the most senior members, and never had bad feedback about the quality of my code. In one of the teams I was the "go to code-quality guy".

Is the problem me? I have friendly relationships with my co-workers and hang out with them outside the office, plus I never had fights or anything like that, and I make a lot of effort not to come out as standoffish or cocky. I rarely cause bugs and when I do I stop everything I'm doing to fix them or help the other person.

I have managers complimenting me, which is weird, because I feel as there is a rift between me and the rest of the team.

This has made me feel that me and my code are deeply unwanted by the team.

Is there any solution to that? Is it all in my head?

Answer 1

I noticed in your post things that could be linked to that situation:

• You are the top reviewer of your team
• You are also one of the most senior member
• Your individual productivity is monitored based on produced code

Therefore

• Your team might be forgetting that they need to do code reviews since you do so much of them
• Your team might think they don't have the skill necessary to "judge" you
• Your team might be thinking they have to focus or prioritize their code production instead.

You shouldn't be afraid to ask your colleagues for review and for potential reason they don't review your code. My approach would be to have a conversation with colleagues in small committee or 1 on 1, because I feel like there would be easier to be honest there, but you could also start asking in meetings e.g. retrospectives and such. Since you have metrics you could ask people that do the least reviews, and be reassuring about the fact it's both easy and appreciated.

Eventually, after you've asked several times to people, they will take good habits of taking a look at reviews and review things by themselves. We often ping ourselves in chat for pending reviews and the load is quite fairly balanced now.

Answer 2

I think the problem here is that team members are each expected to deliver their own work, instead of having a common goal of delivery.

If each engineer has her own piece of code to write and deliver, and is measured on achieving this (and not on achieving a team goal), then of course there is no incentive to review other people's code changes.

I'm not sure what you as a single person can do to change this, though – this needs to be approached from an organizational point (i.e. your manager, or a product owner or similar). The team needs to have clear priorities on what is most important, and then those delivery bits will need to be reviewed before working on other code.

Answer 3

Go to your coworkers' desks and ask them if they are available for a code review. For short ones they will most likely agree, for longer ones they may ask to do it later, take the opportunity to specify a time slot for your review.

If you really need to you can formally set up a meeting, it can be usefully to both ensure the code review take place and prevent you or you reviewer being bothered by other people.

Answer 4

Firstly, don't internalize the lack of code review as being a problem your team has with you. It seems like you're thinking that the lack of code review is a personal slight against you, which likely isn't the case. Sometimes people are busy or they don't see the benefit of reviewing your code. It isn't an attack.

Second, if you see a need for code reviews and the team isn't committing the time, speak to your manager. A few places I've worked had a set time every week for code reviews, and we would knock out a few of the more critical reviews as a team for larger or higher priority changes. Smaller ones we could review in pairs as our schedules permitted. But we never allowed code to be merged without a review.

Speak to your manager about establishing a code review cadence that works for your team, and be open that your manager might not see it as an important change to implement. To some managers, code reviews aren't important until they see how it can improve the bottom line.

Answer 5

It's not you, the issue is the team culture. By by-passing code reviews, the team does not follow software engineering's practices and don't understand why they are important. This is a critical issue that needs a change ASAP. These points need to be tackled:

• Why it's important.
• For who? and who should do it? I would clarify that code review should not only be done by seniors and that seniors also need their code to be reviewed. I have seen teams who introduced git enforcement rules like protected branch that needs two reviewers.
• How to code review and how to give appropriate feedback.

The expectations should be clear. These points need to be presented to the team (E.g. meeting, lunch & learn). Change is hard, be prepared and seek allies.

Answer 6

What has worked for me is to explicitly assign someone for the review. In our current project, we even go so far as to reassign the issue in JIRA to the reviewer. If you can't assign this to another person, assign it to the one who can (the project manager). It doesn't matter that the person actually does the review, it matters that the person takes care a review is done.

Assigning a specific person means that this is now, traceably, a todo of that person. I find that even for me, who does most reviews anyways, it helps if review tasks are assigned to me - it makes it clear that a task is in a state for review, and that progress is blocked by me right now / that I am responsible right now.

Answer 7

I was in a very similar situation. Quality guy, managers complimenting, meticulously taking care of delivery, good relationship with people outside work.

It was on top of my priority list to be a great performer. One day a new guy in my team told me that he heard that I'm an a**hole but everybody loves me. Honest 360 degree feedback. :)

What I realized then was that I was so obsessed with perfection that people were my second priority. Still a high priority, I did whatever I could for the people, except sacrificing quality.

People feel it. Even if it's not conscious on my part, I walked tall like quality was my second name. I realized that this makes people feel secondary or inferior.

I put people as my first priority for real immediately, because that was my intention anyway, I just did not see that I'm not there. Life improved a lot, people built so many new skills, team got great, quality was strong and steady.

This might not apply to your situation, I just leave it here, maybe it helps.

Answer 8

Consensus

You can't get team members to review your code if your team doesn't agree that code reviews are valuable. I'd say code review is one of the three things that most developers don't do enough of. So the first step is to simply talk about code review in a meeting with your whole team (including managers). Ask them what they think about it, whether it's valuable, and how often it should be done.

I'll bet good money that at least half of them think it's a waste of their time. If so, your first step is education. You need to pull up reviews where valuable comments were made, especially if someone caught an actual bug before merge (you shouldn't sell CR as a bug-finding tool, but it is a nice bonus when it happens). If your coworkers lack experience, this might be a hard sell. Even so, it's something you should gently raise as often as possible, so they eventually absorb the idea that CR -> better quality -> better code -> fewer bugs -> better Quality of Life for devs.

Enforcement

Once you get the team to at least pay lip service to the idea that CR is valuable, you then need to make a mechanism for enforcing it. Ideally, your version control system does this (trivial to configure on GitHub, for instance). If nobody can merge without getting a CR, then hopefully everyone is bottlenecked on it, and is thus incentivized to clear out the backlog. When this happens, you need to do something which may at first seem infinitely difficult for you: you need to do nothing. There is an average number of PRs produced per dev, and each dev therefore needs to perform that number of CRs to have an equal distribution of work. You need to estimate that number and do no more than that many CRs.

Soon, everyone on the team will complain that there is a huge backlog of PRs waiting to merge, and you need to make it clear that the whole team needs to carry the load of reviewing them. Gently remind them that they agreed on the value, and resist any calls to remove the CR lock on merges. Most likely, they will do CRs grudgingly and sporadically. To help assign accountability, I created a tool for my team which did a very basic round-robin assignment of PRs to devs. Everyone could see who was not doing reviews, and everyone knew who to talk to if their PR was blocked on review. Just having this visibility helps enforce the desired behavior.

Also, seniority really has little to do with this phenomenon. I see it across devs of all skill levels. Many times it is the junior devs who grew up in a more modern software culture who embrace code reviews, while senior devs who are used to merging without review resist it. So, really, it is just an individual preference which is unfortunately pervasive throughout the industry.

Expectations

Finally, you need to get past the idea that CRs are for finding defects. Code review should be primarily about distributing knowledge. Just having someone else read your change makes another person on the team aware of the change, and more likely that the team notices potential design conflicts with concurrent or imminent changes. It's also for education. Junior devs should get just as much out of performing a CR as seniors, but of a different nature. You should teach the juniors that if they have no critiques of the code they are reviewing, then they should ask questions. Surely they are learning something new here and there, or they see something unexpected that was done differently than they would do it. Asking these questions and getting good answers helps them learn which practices are specific to your company vs. general software engineering best practices.

If anyone expresses reluctance or hesitation to perform reviews, especially of your code, then just offer to pair-review with them. Tell them the things you look for, and ask them to explain your code to you. If their explanation misses something interesting, ask leading questions to highlight your point. If they fail to understand some bit thoroughly, then ask them to put a question on the review to show that your code was not as self-documenting as it could have been. Then go through one of their PRs or someone else's, and demonstrate the same principles. Show by example that a CR doesn't need to be intimidating and that it can be a valuable learning experience for everyone.

Answer 9

You mentioned in the comments that you're working on some more complicated stuff than the rest of your colleagues.

This looks like a double danger: your code doesn't get reviewed, and other people don't know how the advanced topic works. If you got hit by a bus, the team would be in trouble.

That could also be the angle to deal with the problem though. Have a talk with your supervisor about how you think this is causing the team to be vulnerable, and request that 1-2 colleagues be chosen to get "initiated" in the difficult stuff you're doing. That makes them the logical choice to review your code, and it also improves your bus factor.

Answer 10

This seems like a process and management issue. First, you work on a team, so you succeed as a team and you fail as a team. If deliveries aren’t met, that’s a team failure. In the past if we had issues with code reviews and peer testing, and we have, we as a team made contracts to agree that all reviews and peer testing had to be done before starting a new dev task and anyone caught breaking the rules got called out. Other times we just assigned those tasks in a round robin manner. You, your team and your manager need to solve this together, but never be afraid to call out an issue especially if deliveries aren’t being met. Be upfront, be direct, but be polite. Also, don’t be afraid to ask people what the issue is. You’d be surprised, most of the time they’ll actually just tell you.

Answer 11

I've both had and experienced this problem - largely the root cause being either or both of

• you produce a large volume of arguably fine code (meaning they have no real input for it beyond trivialities like grammatical mistakes)
• your code style or the source language is very unfamiliar (meaning they are not confident in their review, worse being the need to read piles of documentation to understand it)

Even if there is a legal reason to require reviewers and not some departmental whim, don't worry directly about their reviews, but instead provide tools to help them review your code

This will give them confidence in the sound-ness of your code as a whole, rather than combing every line for absent errors

• write tests for your code and request reviews on the tests .. try to write a good description around the purpose of the tests
• ask for and work with your boss(?) to create work items for tests against specific functionality
• if possible, collect coverage to prove that the tests do cover all the branches you expect

This will in turn make it far easier for your coworkers to review what you've written, because having contributed to the tests and seeing the good result, they'll have confidence it works and understand how to objectively say it does!

Answer 12

It may not be you, or the process, or busyness. It may simply be that a lot of programmers don't like to do inspections. Some don't believe in them, some don't want to take the time, some know that they aren't that good at them.

There are people like you (and me) that are very willing to dedicate a decent amount of time to inspect a change correctly. There are reasons we do that. I can't say your reasons, but mine include things like:
I inspect better than most designers I've ever worked with,
I spell and read better than most so even your comments will be corrected,
I'm defensive about other people making bad changes in the system that might affect me, or even worse our customers,
my early history included organizing and chairing the old fashioned sit down group inspections which gave me a remarkably broad exposure to our code so I recognize the value of seeing other people's code.

Learn, protect, team. Useful values from participating.

You probably know who the other best inspectors are. They likely share your interest in inspecting, at least a little bit. See if you can figure out why they do it or why they are good at it, and appeal to those instincts. "I've made some changes in this tricky bit of code and I need someone good to inspect it." Or if they are just team players "I need this inspected in [a specific time frame]. Could you help me out?"

Answer 13

I completely agree with what has been said regarding reluctance to review complicated stuff and lack of assigning somebody to be responsible - and I've seen nothing in your question to indicate that you and your code is unwanted by your team! I rather think that your team has no clear processes on how to handle reviews.

We have talked a lot about reviews on my team. Maybe some of our conclusions could be helpful to you and your team:

• What do we want out of a review? We are not (primarily, at least) looking for the small details. We want to catch pitfalls and bad architectural choices and the like. We also want to exchange good ideas and improvements we might be able to spot, but this is secondary.
• Because this is what we want, it would take way too long to do the review without the author. It would be like having to think all the authors thoughts all over, but without the benefit of gaining experience during development!
• Even if the commit is fairly simple, it doesn't have to be scattered across very many files before it becomes hard to figure out where to start and where to end - especially if this is not "your usual area of the code"
• We are experienced developers, and honestly, we usually have a pretty good feeling about what is danger area and what is probably trivial in what we just did.
• Reviews are an excellent opportunity to share knowledge!
• Our solution: The author must ask one or more team mates to come for a review. This can be at the desk or in a meeting, depending on the scope. The author must present what they did, what choices and compromises where made and why - and then the team mates provide feedback. This way the responsibility falls on the author and the reviewer doesn't have to understand everything on their own: they have it presented by someone who has an overview and can answer any questions that may pop up along they way.

I will add that, on a personal level, we are a very well functioning team with a high level of trust. We do not take feedback personally and we present it in a positive, disarming manner. Also, if we work on complicated stuff, we spar and discuss along the way. This way we have a conceptual overview of what the others are working on even before we have to review the actual code.

Even if your team does not come up with a shared strategy for reviews, you can take ideas from this, if you think they will work for you: spar during development, invite team mates for reviews where you present - and do make sure you tell them about doubts and compromises you've had along the way. They may actually be a little intimidated by you if you are very good, so if you show them you have doubts too, they may become more at ease with the reviews.

Answer 14

• Bring that up during regular team meeting (if you have a daily standup or similar, for example):
  • "Whatever task I'm working on is code complete, I need to get it reviewed."
• Right that moment, find someone to do it:
  • "Who can take care of it/who has time/who can I assign it to?"
• Get verbal commitment, if possible a timeline (before lunch, by 3, later in the day, tomorrow, ...)
  • "Thanks, I'll merge it afterward, so we can meet such and such deadline"
• Make that visible in whatever system you are using (e.g., in bitbucket, assign the person as reviewer)
• Bring this up again during stand-up if your code hasn't been reviewed.
  • "Guys, this is blocking me, it needs to be reviewed otherwise feature A can't be shipped next week"

Later, have a discussion during retro (if you have that) or an ad-hoc meeting to explain that code review is part of the work of everyone, that it's a good way to know what others are working on, to learn how they work, and that it's necessary for the team and in order to deliver anything, and so on - I assume you know what code reviews are good for :)