Live data from Hacker News

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

news.ycombinator.com

21–30 of 34 posts

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

#22
post #14
post #8

Write a clear guideline, and ask team to follow it. During the code review, you only ask to stick to guidelines (that everyone agreed on). This way programmers won't feel like your subjective opinion is affecting their work.

This is solving a people problem with a process. That doesn't work very well, in the long run.

This can be done without it being a burdensome process or weaponized against an individual. Developing a team agreement with the team can be a powerful tool. It's important the team itself is the author, that everyone either agrees with a proposal or will go along with the rest of the team (if they won't, then its on them to form a counter-proposal), and that when the environment changes the team can revise it.

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

#23
post #16
post #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…

Also, "you should code like that because that's how we code here" isn't a good reason. If you don't have a good rationale for the changes, I would consider not making them.

On the contrary, that is a very good reason, and sometimes the only one that matters.

In any group of "n" developers there are likely to be "n+m" different ways to do the same thing. This increases the "cognitive load" to understand others' code (a fancy way of saying it makes the code hard to understand).

Personal preference is not a valid reason to eschew standards, even (especially) de-facto ones.

If there is a "better" way to accomplish a particular task, then the entire team should adopt it (and go back and change existing code to conform to the "better" approach). If that's not worth doing, then leave well-enough alone. (Another way of saying "If it ain't broke, don't fix it").

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

#25
It depends on how much skin in the game you have as a reviewer. If you have to or will have to maintain the same codebase (sounds like in your case you do), then you have every moral right to reject the code and refuse to ship it. If you're literally not allowed to reject the code, escalate to your common manager. If it goes wrong later, at least you're on record objecting, and this could also be a signal to get a new job if your manager can't even handle this.

And if you don't have skin in the game (ie, process just requires "someone" to review it even if not on the same team), then whatever, give them the advice and let them burn themselves if they wish.

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

#26
post #16

Earlier quoted context omitted.

Also, "you should code like that because that's how we code here" isn't a good reason. If you don't have a good rationale for the changes, I would consider not making them.

On the contrary, that is a very good reason, and sometimes the only one that matters. In any group of "n" developers there are likely to be "n+m" different ways to do the same thing. This increases the "cognitive load" to understand others' code (a fancy way of saying it makes the code hard to understand). Personal preference is not a valid reason to eschew standards, even (especially) de-facto ones. If there is a "b…

Right, but simply saying, "that's the way we do it here" doesn't communicate all that. Hence, "that's the way we do it here" is not a good reason. "That's the way we do it here, and here is why we do it like that..." is how it should be phrased.

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

#27

Earlier quoted context omitted.

On the contrary, that is a very good reason, and sometimes the only one that matters. In any group of "n" developers there are likely to be "n+m" different ways to do the same thing. This increases the "cognitive load" to understand others' code (a fancy way of saying it makes the code hard to understand). Personal preference is not a valid reason to eschew standards, even (especially) de-facto ones. If there is a "b…

Right, but simply saying, "that's the way we do it here" doesn't communicate all that. Hence, "that's the way we do it here" is not a good reason. "That's the way we do it here, and here is why we do it like that..." is how it should be phrased.

Agreed -- if you can't explain why "that's the way we do it here", then that's a problem.

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

#28
post #14
post #8

Write a clear guideline, and ask team to follow it. During the code review, you only ask to stick to guidelines (that everyone agreed on). This way programmers won't feel like your subjective opinion is affecting their work.

This is solving a people problem with a process. That doesn't work very well, in the long run.

Training code reviews into a process does work VERY well in the long run. See Code Complete.

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

#29
'We' is a bit vague here. Are you this guy's boss?

If not, you shouldn't start a feud with a peer. It doesn't do anyone any favors. Get on with your job and let everyone else get on with theirs.

If so, decide whether the issues in question are really that important. If they aren't, cross them off the list of rules to follow. If they are, order him to do things the company way. If he refuses, fire him for insubordination.

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

#30
post #8

Write a clear guideline, and ask team to follow it. During the code review, you only ask to stick to guidelines (that everyone agreed on). This way programmers won't feel like your subjective opinion is affecting their work.

Its more than guidelines. Frankly the guidelines are arbitrary, and people's ability to find flaws and apply them just as arbitrary. What seems to happen quite frequently is that the guidelines just end up being used as expedient ways to say "I'm busy, go away" or "I don't like your patch, but can't find anything wrong with it" or any number of other non productive things. Reformatting code by hand is tedious, and every second typing "add a space here" in the code review subtracts from a second of understanding and analyzing the functionality of the code.

Which is why if you have coding guidelines they should be enforced by automated tools. Either the code passes the tool or it doesn't, no on is allowed to nitpick brace placement, spacing, or whatever. Use something like clang-format or whatnot and stick to it.

Post reply on HN