Team doesn't take my code review comments seriously

Question

I am a mid level software developer in a small software company. I have been here for the last 8 months and due to my approach with code reviews I was added to the mandatory reviewers list early on.

I came here from a fairly large organization where the turnover rate was very low and every developer was extremely involved in the software development process. Coding standards were followed religiously and code quality was everything.

I came to this company to learn something different, being smaller company I thought I would learn more than I did back at my last job. Since, a few months, I have noticed a careless attitude among my team members when it comes to documentation, code quality and the approach towards code reviews is appalling. I being one of the reviewers on the code reviews, it is very obvious to me. I have to constantly remind them to follow the agreed upon code standards, to document stuff and just take ownership. I am not in a position to ask them to do it explicitly. Their attitude is starting to annoy me and almost stress me out. This attitude is same throughout all levels be it a junior developer or a more senior developer than me.

I cannot simply ignore the basic mistakes that are made in the reviews and let that code go in the production. One bad coder, but senior developer, came to me to ask me to back off on code reviews.

I have already talked to my manager (who is also their manager) about all this, but from his response it sounded to me like he has no control over them (he started a few weeks after me) so he is relatively new.

If you were in my situation, what would you do? Would you change your attitude to not care anymore?

Some e.g. of issues that I see in the code reviews:

1. Newly created unused methods/variables that are left as it is.

2. Naming conventions for new classes, properties, methods not followed. Properties for instance are left all lower case.

3. Obvious bugs are ignored and they get into arguments like the the back off comment.

Answer 1

Their attitude is starting to annoy me and almost stress me out. This attitude is same throughout all levels be it a junior developer or a more senior developer than me.

I don't know if it is the culture or me being a female in a male dominated team that is causing the friction.

To me this appears to be a culture mismatch.

You came from a large company where folks generally followed the rules. You are now in a smaller, more free-wheeling company. And it's bothering you.

I have already talked to my manager (who is also their manager) about all this, but from his response it sounded to me like he has no control over them (he started a few weeks after me) so he is relatively new.

If you were in my situation, what would you do?

First I would probably ask to be removed from the code review team since that seems to be at the core of your stress.

Then I would either learn to accept the culture as it is while still doing excellent work, or find a new company (perhaps a larger, stricter company that follows more formal rules), and leave this one.

Would you change your attitude to not care anymore?

Absolutely not.

I always cared. If the culture around me was such that to get along, I would have to stop caring, I left.

Answer 2

To summarize the question, as it reads now:

1. I want to achieve consistency in the code and avoid stupid bugs in production
2. Part of my job is code reviewer
3. I have to constantly remind them to follow the agreed upon code standards
4. I cannot simply ignore the basic mistakes
5. My manager has no control over team

Ask your manager what is your job exactly. Is it "code quality advocate" or "code reviewer"? It is probably latter, so drop the (3) behavior.

What is the process of code review? Is it commit->review->comment/accept? Then do just that. Refer all grief from the team to your manager. They have appointed you to do an important task, you seem to have appropriate background. Their job is to help you do these tasks.

There is a chance you are doing not a very good job, for example, hanging on indentation, naming conventions, or something else inappropriate for current state of project or product. But that is the problem for your manager to explain to you. If there are written documents, try to link them in your comments, and go together with manager through the guidelines to figure out how you can do your job better.

Answer 3

A technical code-reviewer gains authority by, essentially, being wise, right, and capable of explaining why he's wise and right. Not by management fiat.

So you're going to have to win hearts and minds.

As a senior code-reviewing guy, I have strong opinions on right and wrong when it comes to code. ("Hell is other people's code", to misquote Sartre).

NEVERTHELESS:

I take great care to distinguish "you must fix this because the code does not work" from "I would prefer a different style" (each of which comes in various degrees of insistence). Focus on what's broken. Over time people voluntarily take up my preferences - because they're good, not because I say so (I'm not omniscient, I've just experienced sufficient crap to have suggestions on how to avoid it).

"Do this because it's the coding standard" is never going to convince anyone. Consistency is a good thing, but nevertheless the rationale for the particular convention is needed. Otherwise consistency is just continuing to do things wrong because you've always done them wrong.

Answer 4

I cannot simply ignore the basic mistakes that are made in the reviews and let that code go in the production.

Then don't. If you found a defect, write it down as a ticket and don't accept the code until it was fixed. I wrote articles under defects until it was fixed.

I am not in a position to ask them to do it explicitly

Then you are not doing a code review.

If you are not their manager, nobody will listen to you - it'a a basic human behavior which most people even proud of. Especially in small companies. As you stated, they even ignore your (male) manager. No need to be stressed about it.

---

However, if I am missing some details and you are certain you've been discriminated by your gender, then they have no right to stress you out due to your gender, and here is the pattern you should follow:

1. Create a ticket or documentation for the defect you found, and document it was ignored by others.
2. Then document a defect which was found by a male developer, and notice it was fixed as requested. (If there are several reviewers as you said, it is better - collect the tickets which are solved and ignored.)
3. Face with your manager with the print out those 2 evidence, and ask for the reason why your defect were ignored and you were behaved differently.

Gender discrimination is a serious issue which should not be treated lightly. You said you are "stressed", which means something is wrong - and don't hesitate to escalate the issue until you are completely satisfied.

Answer 5

Ask your manager what is expected of you in code-reviews (possibly with some anonymised examples from your reviews), get it in documented publicly and specifically, and follow that as best you can. If they continue to not follow these standards, it will be up to your manager to enforce the rules, not your problem.

If your standards are higher than those of your manager, then you'll have to relax your standards. It's a shame, but importantly, the issue is taken out of your hands. Unclear expectations are causing you undue stress at work, you should put pressure on your manager to resolve the problem.

Answer 6

I think the focus on what your authority is and how you get people to comply is mistaken.

The purpose of code review is to catch bugs and improve code quality, not to produce perfect code. Insisting that code is perfect before you sign off on code review is the number one way to foster bad feelings and create a stalemate, where you don't sign off on code and the developers refuse to listen to you.

Above all, code review is a collaborative, team process. It needs to be done in a way that fosters team development and relationships, or at a minimum does no harm. Otherwise you'll end up with a team that hates each other and then no software will be developed, good code or not. Thus, when you code review the first thing you always have to do is Remember The Human:

Bring code up a letter grade or two, not from an F to an A

People who produce F-quality code aren't doing it on purpose. Either they're inexperienced or they just don't know better. You can't take a junior developer (or a senior developer with a lifetime of bad habits) and expect them to change all their habits in a day. Moreover, it's an unreasonable standard as no code is ever really perfect: even you make mistakes and miss things. Focus on two or three major deficiencies and address those in your code review, so that the code goes from a F to a C. Trust that over time your developers will get better and once they're producing C level code then you can worry about going to a B or an A.

Tie feedback to principles, not opinions

People have different opinions on how to build code- that's fine. If you insist that things are changed just because you would have done things differently then your feedback comes off as arbitrary and meaningless. Instead, tie feedback to specific principles: "this isn't scalable", "this isn't modular", "too much is happening in this one class or function", "the style guide says to do it this way."

Frame feedback as requests, not commands

You wouldn't tell your coworker to go get you a coffee. Use inclusive language, "We should add some error checking here."

Split large reviews into small reviews

Encourage your developers to submit small changesets and review them quickly. If you get 500 or 1000 lines of code and you come back a week later with five pages of changes to make it's incredibly demoralizing. If your developers know that small changes will be approved that afternoon or the next day then you will get small changesets and your feedback will automatically be small and manageable.

Don't hold up approval over trivial changes

Your co-workers are professionals. Trust their professionalism. If you only have a few comments say "X, Y, and Z are minor changes that would improve things, but I'm approving this change for now."

Offer sincere praise where possible

But only if it is truly sincere. You don't want to be the person who only has bad things to say. That person is just looking for reasons to shoot other people down. You want to be a mentor.

Answer 7

You are trying to change an embedded way of doing things which probably arose due to the incentive structure of the company

My company technically has coding standards, code review, documentation plans, etc. Whether anyone follows them depends on how close we are to the end of the sprint. Up to the end of the first 5 days, there will be unit tests, code review, and SQLs will be organized in their tickets. The remaining sprint time, things get iffy. Bugs in the frontend are generally of greater concern than bugs in the backend.

Why is that? The decision-makers are non-technical and generally prioritize what the customer sees and the features get out the door. We also have external clients we need to satisfy. I haven't had a formal performance review yet, but the corporate goal with regard to software seems to be very much about getting features to production. We have all manner of older software that requires constant software developer support, so this has been the attitude for a long time.

The challenge is that things like code review and testing are not institutional goals but rather just team goals and the institution takes precedence.

Whether management realizes it or not at your company, they are probably the ones encouraging that behavior through what they reward, what they complain about, and what they prioritize.

There is very little you can do about it as you are essentially going against management here.

Answer 8

I very much like Joe's answer, but I'll expand on a few very alarming signs to me.

I have to constantly remind them to follow the agreed upon code standards, to document stuff and just take ownership.

When devs ignore coding standards it's rarely because they mischievously want to be sinister, almost always it's because the coding standards are unfit for purpose. What did you do towards resolving, or at least trying to figure out the core of the problem, besides constantly reminding people to follow them?

I am not in a position to ask them to do it explicitly and hence they seem to think I am the mom that they can just ignore when she yells.

Code reviews in teams are not seminars where one person tells another how to do things. It's a collaboration towards achieving better quality by having an open discussion among people working on the project. If you are acting like a nagging mom, rather than taking the time to explain and seek consensus with the other developers, why would you be surprised that they will simply start to ignore your feedback?

As you point out yourself, you are not their boss, you are just another developer, so act that way and if you want to convince someone to do something your way, use logic, charm and reason, rather than the stick which you were never given.

I cannot simply ignore the basic mistakes that are made in the reviews and let that code go in the production. One bad coder, but senior developer, came to me to ask me to back off on code reviews.

That's a sign that you are at least partially the problem. Do not ignore it when someone senior on the team takes the time to give you brunt feedback like that. There clearly is something wrong with the way you are coming across, at least in context of this very team.

I don't know if it is the culture or me being a female in a male dominated team that is causing the friction.

I don't know where do you even got this idea from. Just because you are a female, doesn't mean that's why less than perfect interactions keep happening to you.

Answer 9

Code quality doesn't matter. Sales do. Making a new product does, because it boosts sales. Re-write the code a hundred times, and your sales won't go up.

---

With that idea celebrated in the back of your mind, you have to also know that code quality does have value. It can allow new products to be built faster, and bugs to be resolved sooner. So some code quality is useful.

I argue that the amount of code quality should be such that the time spent on code quality was slightly less than the time saved when adding new features (due to the improved code quality).

---

In a small company, you need to be a little thoughtful about if your code quality arguments are hurting growth. A small but bad codebase can have things added to it pretty quickly, until it hits some point that it is no longer small and things are no longer added quickly. It's at this point that code quality matters - because code quality has a definite result (less time adding new things).

But until that time, while the codebase is relatively small, code quality adds only marginal value. So, to make your argument about code quality, you might want to consider what business objectives/bugs are the most pressing now and in the next year, and make an argument for particular code quality (reuse, naming, clarity, whatever) in those areas. You can then present this to your manager - "hey, the xyz component is seen as the big thing for our product, we should be proactive now to keep it cleanish so it doesn't cause problems later".

You won't be able to outright create code-quality everywhere, but as the idea sinks in, it will be easier to maintain.

---

There will always be sloppy code, (and that's why full integration tests are so useful - because you can clean it up), but it can generally be cleaned up. Code quality arguments should exist in the context of increased sales, otherwise they are purely theoretical.