HACKER Q&A
📣 MichaelMoser123

Is there some unwritten code of conduct for code reviews?


Code reviews are all the rage these days: I am getting code reviews at work from people whom I don't even know; Is there some accepted code of conduct for this kind of situations? I mean is one supposed to just do what any reviewer is suggesting, or is there an acceptable way to resolve differences of opinion?


  👤 jamessb Accepted Answer ✓
Google's engineering practice documentation has a section on 'The Standard of Code Review': https://google.github.io/eng-practices/review/reviewer/stand...

It includes this important rule:

> In general, reviewers should favor approving a CL [changelist - equivalent to a patch] once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn’t perfect.


👤 noir_lord
In theory a code review should be a conversation in which you reach consensus.

The goal should be that the reviewer understands the code and what it does and is satisfied with the other things.

In reality what often happens is a busy programmer looks at it, spots that there is a { in the wrong place, flags that and considers his/her job done and moves on.

If someone flags something in a way that you disagree/or don't understand the best approach is "Can you explain why you think your approach is better than mine and what the trade-offs are?" in which case you'll either get an answer that is satisfactory and accept their changes or not in which case you then have to escalate it again.

If you think of it as asynchronous pair programming rather than a chance for someone to nitpick everything you do then it's more obvious how to respond.


👤 codingdave
There isn't one standard answer. The overall idea is to get another set of eyes on any code before it hits production, to help catch places where it could be improve, catch bugs early, help each other find better ways of doing things, etc.

In practice, some teams do this very well and it an ongoing discussion of good practices that makes everyone better. Other teams nitpick over trivia, asking everyone else to code it like they would have. Some teams require an actual approval, others just toss all the code up for a while before merging it whether or not it got reviewed. Sometimes a lead reviews it all and say yea/nay, other times it is a teaching tool to expose people to new areas of the code before asking them to help out in that area.

In other words, no - there is no standard code of conduct across the industry. But there should be some common understanding of how your team, specifically, is doing reviews, and what expectations exist.


👤 olingern
Code review, IMO, is specific to an org and how it wants the culture and codebase to mature overtime.

Careless, quick reviews are going to foster an environment where PR authors do not feel adequately supported where as pedantic, nit-picking reviews are going to exhaust.

As for the semantics, it's often about being idiomatic in the language and adhering to good design, i.e. if some requirement changes -- can this change with it?