6 ms·

How to Do Code Reviews

56 points by kevination 7y ago

pc86 7y ago
The title of the article is "How to do code reviews" yet all the content is around why code reviews are done and what the goals are. And the content itself is pretty light - the second half is just rehashing the first half, which are all bullet points. I'm curious how it made it to the front page and if some of the folks who thought it was useful could comment about how it helped them understand how to do a code review?
- finaliteration 7y ago
  Agreed. I found the referenced article[0] far more useful as a guide for how rather than why. [0](https://kickstarter.engineering/a-guide-to-mindful-communication-in-code-reviews-48aab5282e5e?gi=62d1198915c3 https://kickstarter.engineering/a-guide-to-mindful-communica...)
- yepthatsreality 7y ago
  Agreed it was a pretty poor read, no matter how many emojis the author uses. The linked material is a bit better. [0] [0] https://kickstarter.engineering/a-guide-to-mindful-communication-in-code-reviews-48aab5282e5e https://kickstarter.engineering/a-guide-to-mindful-communica...
- n4r9 7y ago
  I agree and even then I think it misses some major goals of code reviews. For example, encouraging thinking about good coding style (as opposed to just "standards"), making use of the language's syntax sugar where it makes things more readable, and the underlying performance. To give a pretty trivial example of syntax sugar, if someone has written in C#: var maxHeight = buildings.Select(b => b.Height).Max(); then I might point out that you can shorten it to var maxHeight = buildings.Max(b => b.Height); whilst keeping it just as readable. Well, more readable because you need to read less characters to easily understand it. It seems pretty nitpicky but it's the kind of thing I'm delighted to be shown myself.
  - deleted 7y ago
    [deleted]
- hinkley 7y ago
  Some of us still recall when code reviews were more of a concept than a practice you actually did at work every day. One of the (unspoken?) tenets of XP is that if you can't have code reviews, break your code into small enough bits that you are comfortable defending each decision individually (rather than rafting them up and trying to get through 10 lines of gold and 20 lines of crap). The blast radius is tiny, and you can't as easiely confuse yourself into thinking buggy code is fine. So there was a time where "how to do code reviews" meant "how do you cause code reviews to occur", not "what processes should you incorporate into your reviews". Maybe OP is caught in a time warp.
jmull 7y ago
This is a nice check-list. A lot of people don't really know this stuff. It's nice to have a concise page to point them to.
kevsim 7y ago
Nice list but for me missing the most difficult pieces - while giving a review, how to not focus on superficial details but instead really take the time to understand the problem and the solution. Making sure you can really understand why things were done the way they were and thinking about whether things could be made more clear, performant, follow best practices more closely, etc - when receiving a review learning that people are reviewing it your code not you, learning how to look forward to reviews because they make your final product better Would love to see advice on the really difficult bits of code review
- arexxbifs 7y ago
  This, and especially the first point, is key to good reviews. It is also why good code reviews take time - sometimes (almost) as long as actually writing the code.
revel 7y ago
This is more of a list of "why" you do code reviews rather than how to do them. In my opinion the way to do code reviews is to be didactic. The point is not just to tell people that there are problems, but to explain why they are problems so that the developer can learn and avoid making similar problems in the future.
- protonimitate 7y ago
  I try to have my team "teach not criticize" when it comes to code reviews. This helps shift the mindset from "what's wrong with this code" to "how can this code be better". Its a subtle difference, but makes a difference in morale and code quality.
Zolomon 7y ago
Steals/Hijacks the back-button via option-left keys, which was quite unexpected.
The_mboga_real 7y ago
Relevant, for Ugandans
justincredible 7y ago
Needs JS, but fails to even mention that.
arminiusreturns 7y ago
The article is more why than how, so I have a question for HN. Is it acceptable for a company to not do code review until late in the funding game (like series E/F)? Like move fast and break things to get MVP going and break a lot of best practices doing it, and at what point should that be reigned in? Imagine you are at a unicorn with physical product already operating in public, and a C laughs at a suggestion of code review and says its too long, hard and expensive. What would you do?
solidist 7y ago
Good post on the what. This one on the human/how: https://dev.to/solidi/be-a-rockstar-at-pull-requests-1e4f https://dev.to/solidi/be-a-rockstar-at-pull-requests-1e4f

56 points by kevination 7y ago

pc86 7y ago

The title of the article is "How to do code reviews" yet all the content is around why code reviews are done and what the goals are. And the content itself is pretty light - the second half is just rehashing the first half, which are all bullet points. I'm curious how it made it to the front page and if some of the folks who thought it was useful could comment about how it helped them understand how to do a code review?

finaliteration 7y ago
Agreed. I found the referenced article[0] far more useful as a guide for how rather than why. [0](https://kickstarter.engineering/a-guide-to-mindful-communication-in-code-reviews-48aab5282e5e?gi=62d1198915c3 https://kickstarter.engineering/a-guide-to-mindful-communica...)
yepthatsreality 7y ago
Agreed it was a pretty poor read, no matter how many emojis the author uses. The linked material is a bit better. [0] [0] https://kickstarter.engineering/a-guide-to-mindful-communication-in-code-reviews-48aab5282e5e https://kickstarter.engineering/a-guide-to-mindful-communica...
n4r9 7y ago
I agree and even then I think it misses some major goals of code reviews. For example, encouraging thinking about good coding style (as opposed to just "standards"), making use of the language's syntax sugar where it makes things more readable, and the underlying performance. To give a pretty trivial example of syntax sugar, if someone has written in C#: var maxHeight = buildings.Select(b => b.Height).Max(); then I might point out that you can shorten it to var maxHeight = buildings.Max(b => b.Height); whilst keeping it just as readable. Well, more readable because you need to read less characters to easily understand it. It seems pretty nitpicky but it's the kind of thing I'm delighted to be shown myself.
- deleted 7y ago
  [deleted]
hinkley 7y ago
Some of us still recall when code reviews were more of a concept than a practice you actually did at work every day. One of the (unspoken?) tenets of XP is that if you can't have code reviews, break your code into small enough bits that you are comfortable defending each decision individually (rather than rafting them up and trying to get through 10 lines of gold and 20 lines of crap). The blast radius is tiny, and you can't as easiely confuse yourself into thinking buggy code is fine. So there was a time where "how to do code reviews" meant "how do you cause code reviews to occur", not "what processes should you incorporate into your reviews". Maybe OP is caught in a time warp.

jmull 7y ago

This is a nice check-list. A lot of people don't really know this stuff. It's nice to have a concise page to point them to.

kevsim 7y ago

Nice list but for me missing the most difficult pieces - while giving a review, how to not focus on superficial details but instead really take the time to understand the problem and the solution. Making sure you can really understand why things were done the way they were and thinking about whether things could be made more clear, performant, follow best practices more closely, etc - when receiving a review learning that people are reviewing it your code not you, learning how to look forward to reviews because they make your final product better Would love to see advice on the really difficult bits of code review

arexxbifs 7y ago
This, and especially the first point, is key to good reviews. It is also why good code reviews take time - sometimes (almost) as long as actually writing the code.

revel 7y ago

This is more of a list of "why" you do code reviews rather than how to do them. In my opinion the way to do code reviews is to be didactic. The point is not just to tell people that there are problems, but to explain why they are problems so that the developer can learn and avoid making similar problems in the future.

protonimitate 7y ago
I try to have my team "teach not criticize" when it comes to code reviews. This helps shift the mindset from "what's wrong with this code" to "how can this code be better". Its a subtle difference, but makes a difference in morale and code quality.

Zolomon 7y ago

Steals/Hijacks the back-button via option-left keys, which was quite unexpected.

The_mboga_real 7y ago

Relevant, for Ugandans

justincredible 7y ago

Needs JS, but fails to even mention that.

arminiusreturns 7y ago

The article is more why than how, so I have a question for HN. Is it acceptable for a company to not do code review until late in the funding game (like series E/F)? Like move fast and break things to get MVP going and break a lot of best practices doing it, and at what point should that be reigned in? Imagine you are at a unicorn with physical product already operating in public, and a C laughs at a suggestion of code review and says its too long, hard and expensive. What would you do?

solidist 7y ago

Good post on the what. This one on the human/how: https://dev.to/solidi/be-a-rockstar-at-pull-requests-1e4f https://dev.to/solidi/be-a-rockstar-at-pull-requests-1e4f