HN Explorer

Ask HN: How do you conduct code reviews?

5 points by weavie ↗ HN
Say you are asked to review a fairly large PR on an area of code you aren't particularly familiar with. I find I get fairly overwhelmed in these scenarios and struggle to truly understand the changes and what they are aiming to do.

What steps do you go through to make sure you are satisfied with the work before giving it that tick?

4 comments

[ 4.8 ms ] story [ 74.7 ms ] thread
Quixotica1
One thing I would say is don’t be afraid to ask for clarification from the PR owner. If someone is reviewing their change, then it’s their responsibility to make sure context/instructions are written well enough that someone who isn’t familiar with that area of code can understand and test it. Usually if I ask enough questions, then they will naturally start being more clear in their PR description.

If your company doesn’t have one, I suggest making a standard PR template that people fill out to help prompt people to give helpful information

JoeMayoBot
One of the things we try to do is keep PRs from getting too big. No standard on what is big or small, but just a general feeling that this is going to take someone more time than normal to review. The person who submits the PR usually lets others know when they know it's large to plan the time. Another thing we do is to take discussions off-line so we can resolve issues quickly (over Zoom or in-person) rather than having long threads. This way, the submitter can explain the code and answer questions quicker. Sometimes there's no way around a big PR, but part of the PR feedback (if the submitter hasn't acknowledged it yet) is that this PR is pretty huge to encourage folks to submit in more digestable chunks.
askafriend
If you feel overwhelmed, ask to run through the code together.
marceloabsousa
For large PRs, I ask the PR author to perform a 'guide' review and then use Reviewpad (http://reviewpad.com/) to be able to nicely iterate over the comments of this guide. This way, I don't need to jump one a call or use loom but I can understand the code changes.