How Chronosphere streamlines the code-review process

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:

• First, with everyone working in different time zones, it can take a whole day to do a single feedback loop on a code review. We want to allow for async code reviews, but also limit the amount of time code is sitting around getting stale.

• Second, there can be many ways to accomplish the same goal and someone might pick a way that another person wouldn’t. We don’t want differences of opinions on how to achieve an outcome if the desired outcome will be achieved either way.

We started off with some guiding principles to help anchor everyone together.

• Assume good intentions, we are all on the same team with common goals.
• Smaller changes are better.
• Reviewing code should be considered high priority in order to prevent staleness.
• Approvals can still need follow up changes.
• Things should have a test associated with them.
• Sometimes we don’t know what works until others try it.

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.

• NIT: to denote trivial preference.
• NB (or none): to denote a requested but not critical change.
• BLOCKING: to denote a change will cause an incident.

Comments for changes should follow one of the following paths:

• Be changed as requested.
• Have context added as to why it isn’t getting changed.
• Agreement to change in a follow-up merge, either as part of the incremental flow or as a separate ticket filed by the commenter or author.

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.