Code Review for Startups
Most would not dispute that code review is a good idea in at least some circumstances. But is it right for my team? If so, how do we do it?
Do we even need code review?
Probably. Software projects usually have three phases: exploration, settlement, and vision.
In the exploration phase, the design of the product or validity of the business is unclear. In the settlement phase, a direction has been determined and we start building for the long term. In the vision phase, complex interdependencies require especially mature development practices. Code review is an investment that pays in as soon as a few weeks. But, for exploratory projects you may not earn returns if you end up quickly throwing most of the code away to start over.
Once you’ve done sufficient exploration and decide to start settling, code review and similar investments such as automated testing will pay in the long run if implemented correctly. Startups may frequently find themselves in the exploration phase, but this is no excuse to cut corners when building for the long haul.
Should my small team really, really, really do code reviews?
How do we do it?
In theory, code review is simple:
- Open a pull request for each change
- Once the pull request is approved by the desired reviewers, it can be merged
But, there’s a few practices you should follow to get the most value from this process.
Keep pull requests small!
The level of effort required to review a pull request increases exponentially with its size. Use this rule of thumb:
💭 “If you think this PR would be easier to review if it was broken into smaller pieces, break it down”
In the interest of facilitating quick and thorough reviews, authors may be asked to break up PR’s that are too big to be reviewed effectively.
The Art of Small Pull Requests
Use small, well-factored, and descriptive commits
To painlessly keep PR’s small and easy to break apart, practice good git hygiene and commit your code frequently, in small chunks with detailed commit messages.
Small, well-factored commits let you use git to its full potential
Git works best when commits are small and isolated. For example, git bisect can search for the commit that introduced a bug by running a test case across the history. Each commit that leaves the app in a broken state when isolated from future changes will cause bisect to halt.
Git — git-bisect Documentation
Descriptive commits help others understand your code
Commit messages should have their first “subject” line limited to 50 characters and include a commit “body” for substantive changes. To add a commit body, just insert two newlines. Here’s an example of a good commit message from the postgres repository:
Future developers may depend on your commit message to understand the reasoning behind your code. git blame can be used to reference this.
💡 Git commit messages will likely last as long as the project, while pull request descriptions and comments will only last as long as the git hosting provider (e.g. Github vs. Gitlab).
How to Write a Git Commit Message
Review and comment your own Pull Requests
Leaving your own comments may be appropriate for providing some extra context that doesn’t deserve a comment in the source itself.
It’s also always a good idea to do your own code review before requesting review from others. You’ll probably find something.
Why You Should Review And Comment Your Own Pull Requests — Leeor Engel
Review others’ pull requests quickly
Pull requests should be reviewed as fast as reasonably possible.
- If you are not in the middle of a focused task, you should do a code review shortly after it comes in.
- One business day is the maximum time it should take to respond to a code review request (i.e. first thing the next morning).
- Following these guidelines means that a typical PR can get multiple rounds of review (if needed) within a single day.
Give effective feedback
Avoid personal, accusatory, or commanding language
❌ “Why would you do it this way?” is mostly not constructive
✅ “It might be better to try [describe alternative approach]. Have you considered this?” is better
❌ “You didn’t close the socket connection” is personal and accusatory
✅ “The socket connection is never closed here” focuses on the code and not the person
❌ “Delete this line” does not justify the suggestion or afford for any misunderstandings
✅ “This variable seems to be unused” is better
Avoid condescending and murky works such as “just”, “easy”, “only”, or “obvious”. It’s usually not as easy as it looks.
Keep in mind that we all have different perspectives and may approach similar problems differently. This is a good thing.
How to Give Respectful and Constructive Code Review Feedback — Dr. McKayla
Note:
Some aspects of the code review process may vary from team-to-team so I’ve excluded them from this guidance. For some finer details on a workflow I commonly use, see: Code review workflow
Appendix: how to break up a big PR
It can be frustrating when a reviewer asks you to break up a large pull request. Especially so if the commit history is not clean. Here’s how to do it.
If your PR is too big and you have many small commits:
- make a new branch off the big one
- Use
git rebase -i [oldest target commit hash]^to select a smaller set of commits - Complete the rebase and open a new PR
Alternatively you can create a new branch off the base and use git cherry-pick to select the desired commits.
✅ This is relatively painless thanks to your good commit hygiene
If your PR is too big and you have a few large commits:
- make a new branch off the big one
- move your changes from the big commits out of staging using e.g.
git reset HEAD^for the most recent commit - use
git add -p [target file]to selectively add back a smaller subset of the changes from each file as needed to staging - Recommit your changes and make a new PR
💡 This is hard because your commits are too big or interdependent
There’s many other ways to do this sort of thing but this is how I do it.
