Chronosphere engineer writes about challenges with the code-review process when everyone is remote, and shares solutions Chronosphere engineers have come up with.
On: Sep 28, 2021
At Chronosphere we have a globally-distributed, remote-first engineering organization which lets us hire amazing and diverse talent from everywhere. While this diversity has lots of benefits, it comes with some downsides that don’t typically need to be addressed in small co-located startups. In this blog, I discuss these challenges and share solutions Chronosphere engineers have come up with as a team.
There are two main challenges we run into when doing code reviews:
We started off with some guiding principles to help anchor everyone together.
Commit messages should describe the change in reasonable detail. The more context, the easier for others to understand the change. Chris Beams writes about how to write a Git commit message here. Automated and trivial changes can be short and simple.
Prior to adding others to a pull request, it is recommended to do a self review of the diff and ensure that the CI pipeline is passing. Think of it as proof reading.
The body of the pull request should provide an overview of the change along with a link to any related docs or tickets. Screenshots for UI changes are strongly encouraged as it gives reviewers a better understanding of the change.
Remember that new employees don’t have all the background or tribal knowledge that you might have. Also, “future you” might not remember what was going on, if you have to look back at the review, so having some context helps everyone.
If a ticket is related to the pull request, make sure to include a reference to it. Pull requests should aim to have small incremental chunks of work. Larger code reviews require more time to do and are harder in general.
You can read more details about optimal pull request size here.
Comments should be clear about the severity and desired changes. If possible, the suggest change feature should be used. Otherwise, example code – or a link to existing code – that demonstrates the change should be provided. Most of our review communication is done async so it is important to make sure things are clear and addressable without having to wait a full day to make progress. After a couple of back-and-forths, comments alone tend to become unproductive, so try hopping on Zoom to figure things out if you find yourself in this situation.
Comments for changes should follow one of the following paths:
Generally we should favor getting something out over perfecting it. Blocking should be rare, and used to denote dangerous changes that would, or could, cause an outage or other incident. Reviewers with blocking issues should reach out to the author on Slack or Zoom to discuss the issue as soon as possible. This helps avoid long async communication delays and letting the code get stale.
Using our “assume good intentions” goal, we’ll trust each other to merge good code and take the necessary followup actions that may be needed in a timely fashion.
Once you are done commenting on the review, if there were no blocking issues it should be approved. This allows changes to be made and merged without another round of back and forth.
Request a demo for an in depth walk through of the platform!