Live data from Hacker News

Ask HN: How to deal with refusal to change code during reviews?

news.ycombinator.com

1–10 of 34 posts

Ask HN: How to deal with refusal to change code during reviews?

#1
This particular programmer has 7-10 years of industry experience in various areas but lacks knowledge about this particular language/framework and doesn't have a lot of experience in day-to-day infrastructure operations.

He will usually dismiss 60-80% of the feedback as not being important, not having an impact, or will flat out say "I will keep the code as-is" without giving a reason.

We want to be an welcoming place and let people learn on the job but we worry that our codebase is becoming a minefield (someone started keeping a document with "future issues" we'll face).

This isn't simply a junior programmer not knowing better. It's seems to be a mix of ignorance and arrogance. We worry he won't be around in an year or so, when things start to break.

Re: Ask HN: How to deal with refusal to change code during reviews?

#3
Does this person work alone, while also being responsible for the changes that are bureaucratically and technically important to the 'system'? That shouldn't have happened, never leave us to ourselves for extended periods. Next time place a pair in that position of power because when you only answer to yourself there are no opposing views.

Re: Ask HN: How to deal with refusal to change code during reviews?

#4
post #2

Reject it. I've done it a few times, there was a fuss, there was a discussion, it got sorted out. At the same time I've had a couple rejected (due to not adopting the reviewer's preferred code style). Same thing happened.

I agree. I would flat out say "I won't accept this change".

Re: Ask HN: How to deal with refusal to change code during reviews?

#5
I have definitely worked with this type of programmer before.

One thing to can do is quantify a code quality metric, for example, test coverage percentage. Then, get the team to agree that all feature branches must maintain or improve that metric to be merged. Maybe treat hot fixes differently depending on the situation.

However, this doesn't necessarily solve the issue if, for example, the implementation is just overly complex, not well designed, or not well decomposed for the problem being solved. There are code quality services that address some of these, but perhaps we need a little more detail on the specifics of your situation.

Unfortunately sometimes I've just had to merge code that's lower than ideal quality, and then hold the engineer who wrote it responsible for fixing it when it breaks.

Most engineers are smart enough to admit when they made a bad assumption or a better solution becomes more obvious in retrospect. Sometimes people just learn at different paces or can be afraid of trying new paradigms inconsistent with how they've been thinking for a while.

As long as they learn eventually, your team is improving, but if they're not interested in learning better solutions, then maybe there are two different cultures in mind. Some devs value shipping faster over writing better code, and having your team on the same page is helpful.

Re: Ask HN: How to deal with refusal to change code during reviews?

#6
post #2

Reject it. I've done it a few times, there was a fuss, there was a discussion, it got sorted out. At the same time I've had a couple rejected (due to not adopting the reviewer's preferred code style). Same thing happened.

This is why it's helpful to have a consistent code style across the team, or at least across the codebase, so that personal opinions in either direction don't cause too much conflict in code review. Some companies value low quality but running code now over better quality code tomorrow.

Re: Ask HN: How to deal with refusal to change code during reviews?

#9
It's not just about "you should code like that because that's how we code here" type of problems. That makes you, the arrogant person in the story. You should create a list of bugs generated by his code. If the list is significant AND if you believe these bugs could be avoided by incorporating %100 of your code review feedback, then setup a 1-1 quality meeting with him and go over the list. Tell him how this could be avoided in the future and that the goal is to stay away from bugs and to focus on fun things. Don't use "you should", use "we should", act like it's a team effort.

If he doesn't accept your invite, or doesn't want to cooperate during this meeting, resend an invite and include your manager and QA and all the other engineers. Update your list of bugs and add bugs from the whole team not only his bugs. Then make it a real team exercise. The problem will raise during this meeting and everyone will clearly see that this dude is a bug-generator.

It is a long process, but that's how you do it professionally. What do you earn from this big effort? People will thank you for taking initiatives and to raise the bar in terms of quality. In other words, you'll get a bonus ;) He will regret not listening to you when he had a chance to fix his mess "secretly".

Post reply on HN