Enterprises are slow.
Something that might be a util.js
file in a startup becomes a Class in an enterprise. It has its own team and documentation. Experts review it, and you feel like you age twice as fast every time you hit "Create Pull Request."
I had a lot of fantasies too, about code reviews in big companies until I joined my first mid-market tech company in May.
I also just got my performance feedback last month. By my company’s custom metrics, I’m acing code reviews, far above average both for comments left and pull requests reviewed. 🏅
As a proud owner of my newest badge of honor, let me talk about my code review mindset and share some of the research I found on big tech and code reviews.
Let’s get started!
Growth Mindset: Turning Code Reviews into Learning Opportunities
Code reviews happen between two engineers - the author and the reviewer.
Wrong.
A code review is a living documentation for the code about to be merged. And as a documentation, it’s read by many people.
Maybe it’s the two of you now, but tell me:
How many times have you wondered about a particular line of code and asked, why does it have to be this way?
Then you look at the blame, find the SHA or the pull request ID, and thoroughly read through the PR.
If you’re lucky, you’ll see discussions between the author and the reviewer, and hopefully, the answer to your question—this is why it has to be this way.
Code reviews, besides verifying no unwanted changes get merged, are about giving away useful information, providing examples, and techniques.
Junior engineers, new people on the team or the next engineers who inherit your code might eventually end up reading your review comments, so think about this.
The Empathy Factor: Crafting Constructive Feedback
Code is not fine wine.
Code is not something you taste as a code sommelier and given what you see and what you personally like, you give it a 👎 or a 👍.
Code exists to serve a business need and solve a problem, so the most important question is: does this work?
Once it works, you have a limited space to exercise your code-tasting skills.
“I’d write this differently” is neither constructive nor helpful. Since mind meld is still WIP, I don’t know whether what I write is up to your taste, but that’s not the point.
Some of you think, yeah, but we have conventions.
Whether you use space or tab or trailing commas is not a convention, it’s a style and you should always automatically enforce styles.
No engineer should ever have to manually check for missing trailing commas.
If you have conventions automate their verification, for example in JavaScript through ESLint.
Beyond Bug-Hunting: Code Reviews as Knowledge Sharing Sessions
Of course, there are exceptions to what I just said.
The code that works might still need some changes.
Imagine a junior developer joining and implementing a client-side fetch for a list of projects in React.
You know you’ll work on a feature in the next couple of days that’ll need the same fetch.
In this case, a comment hinting that the fetch could be implemented as a reusable custom React Hook is completely justified and is often the case in how these fetches are implemented in large-scale front-end apps.
Whether you’re already working on such apps or just at positions at companies working on big software, you need to be aware of custom React Hooks.
Lucky you, this week I’m running a sale on my course
Click the link or use the promo code 5F461D9875FE1A463095.
Streamlining the Process: Best Practices for Efficient Reviews
Here’s the universal formula for the number of comments in pull requests:
10 lines of code = 10 issues
4567 lines of code = LGTM
Smaller pull requests get better reviews.
You should aim for something around 500 line changes.
Except if it’s a refactor and the set of changes is more repetitive - for example removing an import from 1000 files.
Do not mix the two.
That makes both the easy changes to be skimmed and the repetitive change more difficult to review. Unrelated changes break the flow as you scroll through the repetitive changes.
I tend to be generous in accepting a pull request.
I see something, that can be done better, but unless the current version affects performance I’m approving it.
Check out my other article about code reviews:
Building Psychological Safety in Code Reviews
Creating pull requests can be stressful, especially if you’re new to a team.
That’s why I consider it crucial to keep these two things in mind:
Every comment should focus on the code and provide new insights, examples, and best practices. Avoid comments like "I don't like this."
Code reviews should never be a place where skilled people try to show off their expertise. Gatekeeping is not helpful and will discourage new team members from working efficiently. They might repeatedly check their pull requests, fearing harsh comments.
Reviews should be completed quickly and shouldn't require approval from a board of engineers.
To my surprise, this is also how big tech companies do it!
A study Modern Code Review: A Case Study at Google, C. Sadowski et al., ICSE-SEIP ’18 reported that:
Developers reported they were happy with the requirement to review code. The majority of changes are small, have one reviewer and no comments other than the authorization to commit. During the week, 70% of changes are committed less than 24 hours after they are mailed out for an initial review.
What do code reviews look like in your organization?
Let me know in the comments!
📰 Weekly shoutout
Here is what I enjoyed reading this week:
📣 Share
There’s no easier way to help this newsletter grow than by sharing it with the world. If you liked it, found something helpful, or you know someone who knows someone to whom this could be helpful, share it:
🏆 Subscribe
Actually, there’s one easier thing you can do to grow and help grow: subscribe to this newsletter. I’ll keep putting in the work and distilling what I learn/learned as a software engineer/consultant. Simply sign up here:
Excellent tips Akos.
Code reviews are always tricky because so much personality stuff can get involved if they aren't handled properly.
I'm sure if people follow this type of code review approach, it's going to benefit all the involved parties: the developer, reviewer and also the organization.
Also, thanks for the mention!
Good article, crisp and precise. Its always about learning new things from review. Its a way to teach juniors as well about logical parts of application how it can improved. It can be in terms of code refactoring as well on how to write better code.