Live data from Hacker News

Ask HN: What tone to use in code review suggestions?

news.ycombinator.com

141–150 of 309 posts

Re: Ask HN: What tone to use in code review suggestions?

#141
As someone on the junior end, the tone is less relevant here than the content, and there is not a lot of content. Why do you want it extracted?

Will we re-use it? Do you just not like blocks of code bigger than X (been given this reason many times)? Do you just feel the need to comment (a lot of devs do and I appreciate it when they are upfront about that rationale and I have done this myself to show that I am paying attention)? Are you concerned about readability?

I need to know for future code reviews, so I can avoid the issue in the future.

Re: Ask HN: What tone to use in code review suggestions?

#143
People can go ahead mod me down for this, but I honestly don't care at this point. Here's the best ways to avoid this:

1.) Quit doing PRs for feedback. Start doing pair or mob programming. Bonus points if you ditch PRs completely and do https://trunkbaseddevelopment.com/ instead while you're at it. Asynchronous code review via PRs is waste (in the Lean sense). 2.) If you still want PRs despite pairing & mobbing (or you're about to tell me you've never done it, you're team won't and/or whatever insert lame excuse here as to slam/dismiss it without trying), spend a faster 5 min via screen share and audio AND CAMERA! Do the PR like an old skool code review session live/remote + camera, but have the author ANNOTATE for their own notes what you've reviewed when they go back to revise. 80% of human communication is non-verbal - interact w/ voice & camera. Help them learn & build confidence, then you won't worry about your writing tone. 3.)

Re: Ask HN: What tone to use in code review suggestions?

#144
I've learned that commands not phrased as such usually aggravate people the most - especially #1 from your list or questions where the one questioning knows the answer and wants to suggest something.

If I don't know how to phrase my comments I just schedule a call and we go over the code together. Much less time spent than on comment ping-pong.

If the code generally works, but other aspects, like maintainability, would benefit from some rework I use the phrase "You can...", e.g. "The second argument to the `map` callback is the element index. You can use it to create this range instead of using an external variable incremented on each step."

The recipient has the possibility, but is not actually forced to do anything, just like with code suggestions. In the vast majority of cases they follow my advice though.

If there's an error or a kludge I use:

-Code suggestions - no comment, so nothing to get upset over and if the recipient is feeling particularly irritable, they can just ignore it.

-Direct language in imperative form, e.g. "make sure that X, otherwise this won't work as intended", "avoid X in Y", "use Z here (documentation link)".

Re: Ask HN: What tone to use in code review suggestions?

#145
post #144

I've learned that commands not phrased as such usually aggravate people the most - especially #1 from your list or questions where the one questioning knows the answer and wants to suggest something. If I don't know how to phrase my comments I just schedule a call and we go over the code together. Much less time spent than on comment ping-pong. If the code generally works, but other aspects, like maintainability, wou…

[deleted]

Re: Ask HN: What tone to use in code review suggestions?

#146
I'm sure this will piss people off: but almost all of the 'code reviews' I've been a part of were a waste of time. People trying to force stylistic changes to how a program works (OPs post is an example) aren't contributing anything constructive. What is worthwhile is trying to find critical security, logical, or performance bugs and reporting those. If you have taste / style concerns about how someone does something yet their solution is still fine then I'd suggest not wasting peoples time. There is nothing worse than nit picking.

Re: Ask HN: What tone to use in code review suggestions?

#149
post #29

There's basically two categories here: - you have an opinion on how things ought to be done and want a dialog - you see some code that's wrong or violates an agreed-upon rule and so it should be changed with no discussion I'd switch the tone based on what you're addressing. Giving a rationale when you share an opinion or point out a mistake also softens the tone (for the better IMO). >* Should we extract this to a se…

> I'd switch the tone based on [whether you want a discussion or are insisting on a change]

I think that's a great example of where "tone" is absolutely not sufficient, especially in the modern world where many readers aren't going to share your first language and won't absorb the nuance.

If there's a situation where your disagreement is absolute, you need to say that factually, ideally with a recipe for how to resolve that included:

"I can't approve this scheme, please do X instead."

"This API isn't OK to use here, you have to..."

"Under no circumstances will I ever approve of a feature that does this."

You can phrase those as nicely as you want, but it's imperative that your disagreement be spelled out explicitly.

Post reply on HN