Comment by ysavir | Hacker News Reader

Comment by ysavir | Hacker News Reader

Remember to be problem oriented rather than solution oriented. If you just tell people "I think this should be the solution" or "you should change it to this solution", you aren't working with them, you're instructing them. And a key to teamwork, especially reviewing, is a cooperative spirit, not an instructional one.

If instead of saying what you would do, or what you think they should do, you express a concern about the code, the tone is completely different. Now it's just saying something like "do you think repeating this across files will be problematic?", which allows the other person to be the determiner. If they agree they may well do exactly what you would have suggested, and if they don't, it can start a discussion that helps bridge together how various team members code. It's win/win.

It also helps people understand what you aim for when writing code, which helps them keep in mind your needs in the future (and vice-versa for you to keep in mind their needs).

> "do you think repeating this across files will be problematic?"

If you write that you clearly think it's problematic but hide it behind an unnatural question. Then either I do what you hint at or you will push / discuss until I agree. I am not the determiner. It sounds like you are schooling me with an awkward back and forth.

Honestly, to me this is beating around the bush and doesn't actually make me confident we can have a real discussion (in the context of that code review).

"Do you perhaps think that doing this <tylically discouraged coding practice> is problematic?" sounds like either you want to passively insult me, or you don't consider me a peer with whom to have an actual discussion on equal footing, or you don't have the guts to share your opinion without contortions.

The wording above could be improved for sure. But the intention is to shift focus away from telling somehow how to solve a problem to forming consensus on whether there's a problem at all. And that starts with the reviewer expressing a concern rather than immediately providing a solution. "Is a change needed?" should come before "this is a needed change".

A better example might be "I'm seeing this same code in various places--do we benefit from keeping them distinct?" or "you took an inheritance approach here, but I'm concerned about the long-term impacts as the codebase changes. How do you think this code compares with a composition approach?".

If in doing so you would think that I'm being sly, that's unfortunate, but there's not much I can do in how you interpret stuff. I can't control for others assuming bad faith. And if there's bad faith, there are probably communication and/or trust problems amongst the team members that are broader than code reviews, and is something that should be tackled directly.

Reply on news.ycombinator.com