Teams sometimes experience issues with bugs in code or pull requests not being merged in a timely manner. Even after they establish a very clear policy on code review and reviews feel like a chore.
At Rietta, we resolve code reviews quickly by making the reviews as painless as possible. We do so by making our pull requests small, single purpose, and informative.
A good pull request should be clear in scope, describe what has changed, and explain why it exists. It should be quick to review, and explain to those unfamiliar the things they need to know.
A great pull request has small, easily digestible commit messages that are rebased to make sense. It has a test suite that covers any added functionality, and continuous integration that ensures it doesn’t break anything. It has a great message that effortlessly conveys what the PR hopes to accomplish. It follows SRP, meaning it has no changes outside the scope of the issue it aims to solve. Stylistic checks are outsourced to a tool such as Rubocop so that code changes discussed within the PR are only on the actual functionality changes. It is visual if necessary, including screenshots of before and after the change.
Reviewing pull requests is hard work. Your top priority is to ensure that the cognitive load of reviewing your PR is as low as possible. Following SRP, there should be nothing outside the scope of what you’re trying to solve. You should get in the habit of
git diffing before staging to ensure that nothing is tracked into the PR be accident. This can mean the difference between embarrassing yourself by accidentally committing a frustrated
notes.txt and not. If you find yourself staging something you didn’t mean to, we have a nice introductory article on git reset to help you get out of that situation.
10 lines of code = 10 issues.— I Am Devloper (@iamdevloper) November 5, 2013
500 lines of code = "looks fine."
Make your PRs as small and self-reliant as you can, hiding additional features behind feature flags and breaking them into their own PRs if necessary. A thousand-line PR can make finding bugs like a needle in a haystack. Breaking up PRs means that your reviewers have an easier job, and make less mistakes, letting less bugs into your system.
Pull requests are vital to the maintenance of both your code base and an engineering culture of excellence. Being intentional in your pull requests instills in your company a culture of software craftsmanship. Take pride in your work and don’t help your co-workers gloss over mistakes – write great pull requests.