How to manage a developer who objects to our code reviews as being "rude, condescending or otherwise toxic?" [closed]

Question

Background: As a developer what should I expect as code review feedback? What if I perceive it as rude, condescending or otherwise toxic?

This developer has become so frustrated with their code review process that they are now escalating the matter to their manager and HR. Their complaints are that their reviewers are under qualified and are often asking the same questions over and over again, even questioning decisions that were made in prior meetings. They seem to be having an unusually emotional response to this, even describing mild panic attacks.

I know a lot of people dislike code reviews and sometimes take criticism personally, but I've never heard of something this extreme.

Ideas I've had:

1. Discipline them and give them very clear feedback that this is part of their job and the expectations are that they will help other, less experienced, developers understand their code. (I don't think this will change their behavior based on how remarkably strong their feelings are)
2. Give them some sort of preferential treatment by either having them only get code reviews from someone who is willing to deal with this behavior. (I think this will cause huge problems in the culture of the development team in that others will resent this and it will only encourage the behavior)
3. Fire them. (I hate this option. It doesn't really solve the problem, it just gives it to their next team.)

I'm not going to build a team of all senior engineers to satisfy one person's complaints, there are several mundane tasks that can be handled by less experienced developers; I'm not willing to pay someone six figures to handle those simple tasks (even if I could afford it, I'm not sure I could find that many of them). Part of our code review process is to improve their skills and let them understand what's going on around them, so abandoning code reviews isn't going to happen either.

Answer 1

I'm pretty sure this retreads ground from previous questions, but having been in the shoes of both sides, I'd give the senior developer some presumption of clue, and work with them to convert their complaints into constructive actionable improvement recommendations, and have everyone follow through on those process improvements, and monitor with each party involved if they're happy things are improving:

1. To be frank, quite a few code reviews are “rude, condescending or otherwise toxic”, if the people doing them are underexperienced, don't communicate well, petty, clueless, lazy, there's a total lack of guidelines, software process, documentation, written minutes, follow-through... it's your job to convert complaints into constructive improvement. I don't get why you seem to automatically doubt everything your developer says and dismiss them as a crybaby, even if they're not saying it in the optimal way, or to the right parties. For the love of God don't expect HR to articulate constructive improvements to your software process. In the history of software I've never heard of this occurring. Unless the code reviews are kindergarten-grade name-calling.

2. Your three suggested options are limited to being harsh, coercive and condescending, I'm frightened there was no option "Actively listen to developer's complaints, and help reframe them into constructive change: 'How can we improve our code review process/ software process? Eliminate repetition? Gather and formalize our knowledge base/ codebase? Improve documentation?'". That's a criticism of your listening style. So you think they should have been able to put up with it because you could... beware of condescending, we all have our weaknesses, check your ego... there are presumably things they can do easily which you might find impossible. If you honestly find it that hard to interact with them, go talk to them privately and clear the air. Constructively. Sometimes some of my more annoying coworkers have taught me useful things. Even if I had to decode their ball of emotions.

3. "even questioning decisions that were made in prior meetings" Well, assuming that's true, who's keeping (concise) minutes of those meetings? in the same documentation system as code reviews? (JIRA? git? Bugzilla? wiki? Mondrian? GoogleDoc?) If no-one, appoint someone ASAP. If someone was doing it but badly, tell them to do it properly, or reassign the ask (but beware of rewarding bad behavior). Again, that's a legitimate constructive complaint/improvement suggestion. Code reviews are not back-door new-hire training sessions. Don't let new-hires push their responsibility for learning onto senior developers and into code reviews. Also, do you have meetings that don't result in decisions, or action items? That's another common organizational antipattern. Is a comment a decision? Is a group vote legitimate? When is the majority wrong? etc.

4. one powerful suggestion (esp. when nerds communicate) is to reframe questions/criticisms to focus on things, not people, and in particular to depersonalize queries/disagreements: e.g. *"Why does your code do X? Why did you implement Y as A instead of B?" -> "What are the reasons for lines LLL doing X? Why is it better to implement Y...?"

5. As a general process thing and to teach everyone that your process is community-owned, have the junior guys (esp.) start reading a bunch of books on improving the coding and reviewing process, e.g.. "Code Complete: A Practical Handbook of Software Construction, Second Edition", esp. Part V. Have them make process improvements. Make them document repetitive questions (have them start a wiki). And so on. Ask the developer if these improvements are helping.

6. Also privately ask your developer (and their manager) to always try to reframe complaints into process change requests ('active listening' is one of many techniques. Basic empathy always helps, too.) They'd probably benefit from some book or course. If the situation got escalated as far as HR, that also tells me the manager involved was not good at handling this sort of thing. So, you all need to learn from this. Don't hang this on developer.

7. It's often hard to disentangle technical problems from people problems, and more so the longer they go unresolved and the more unfairly-treated or dismissed people feel. You all should collectively never have let things go so far...

Answer 2

Took me a while to grasp what this is all about: You have a senior developer who writes code, and there are no complaints about this code. Code is reviewed, as it should be, and often the code of the senior developer is reviewed by much less experienced developers.

And what he complains about is that these less experienced developers are clueless, ask the same questions again and again, can't remember what was said in meetings, and are generally a pain to someone who doesn't suffer fools gladly.

This sounds bad. Your problem is that you have devised a method for training your junior people that is absolutely inappropriate. The role of a reviewer is to improve the code that is reviewed. Code reviews are not for training people. Training is for training people. And senior developers are not trainers. If you want someone to give training to your junior developers, hire a trainer. Not a senior software developer.

And if you don't want to hire a trainer, then tell your senior developers to give training to the juniors, don't abuse the code review process for this.

Maybe you just should fire this developer. He will find a new job in a better working environment, and everyone will be happier. Your only problem is that you are now short one senior developer. And the other senior developers may be more resistant to the frustration that you create for them, but eventually they will leave as well.

Answer 3

Upon reflection, I think that the right answer here is to train the team on how to give and receive code reviews. I've expected people to understand them conceptually and to be able to implement them from my own explanations of the process.

Going forward, here is what I'll propose:
In our on-boarding of new developers, we inform them that we have a code review process and we even have a written policy on it that we have everyone read. We review that document annually and have everyone confirm that they've read the updated document afterwards.

If that written document proves insufficient, I'll ask for feedback from the rest of the team and possibly give a presentation on the topic.

A special thanks to @Lilienthal for pointing out that I might have overlooked the possibility that there are deficiencies in our existing processes, something I had dismissed outright but really shouldn't have.

Answer 4

Without code reviews, his colleagues will just change his code (without telling him), or they'll be too afraid to touch his side of the code base.

In an agile shop, neither of those options are desirable.

Code Reviews are for getting everyone up to speed on the same code base. They're a form of cross-functional decision-making and cross-functional training.

The potential disagreements and misunderstandings that code reviews bring up to the surface are not created by the code reviews.

Without code reviews, those disagreements and misunderstandings would just end up rearing their ugly heads at later dates (except the original developer who wrote the original code in question won't be there to explain his side).

That being said from the comments of your developer:

we do them individually from our desks. I always tell everyone I'm open to discussing things in more detail but we end up just trying to type a million lines of text into visual studio/tfs system – DefunctNinja

What DefunctNinja seems to be describing are not code reviews at all, and perhaps that's the problem.

For one thing, code reviews are supposed to be limited in scope and limited in time. And to me, typing "a million lines of text" sounds like those sessions are going on for way too long.

Also since those sessions are held so casually, it's doubtful that the other developers prepare for them by reading the code and annotating the reviewed code in advance either.

They probably just blurt out the first thing that comes to mind when reading the code over your video-conferencing session, hence their lack of filters and tact. And so that's not really a carefully crafted code review session at all, but more of an endless "bike shed" second-guessing discussion instead.

Parkinson shows how you can go in to the board of directors and get approval for building a multi-million or even billion dollar atomic power plant, but if you want to build a bike shed you will be tangled up in endless discussions.

Parkinson explains that this is because an atomic plant is so vast, so expensive and so complicated that people cannot grasp it, and rather than try, they fall back on the assumption that somebody else checked all the details before it got this far. Richard P. Feynmann gives a couple of interesting, and very much to the point, examples relating to Los Alamos in his books.

A bike shed on the other hand. Anyone can build one of those over a weekend, and still have time to watch the game on TV. So no matter how well prepared, no matter how reasonable you are with your proposal, somebody will seize the chance to show that he is doing his job, that he is paying attention, that he is here.

In Denmark we call it "setting your fingerprint". It is about personal pride and prestige, it is about being able to point somewhere and say "There! I did that." It is a strong trait in politicians, but present in most people given the chance. Just think about footsteps in wet cement.

source