+1 with comments

At one of the big tech companies I worked at, we had comprehensive documentation around code review best practices & coding style guides. This documentation has been open sourced and is publicly available (see code review best practices, java style guide etc). The standards incorporate best practices across coding languages/code reviews and are updated fairly frequently to incorporate new features as languages evolve. The big tech companies have a bunch of pretty smart folks coming up with these guidelines. However, like most things in software development, there is a caveat — the standards are ultimately a set of recommendations and can sometimes be subjective. For e.g. should the standard be to use tabs or spaces? (who are we kidding, tabs are clearly superior).

However, in spite of them seeming somewhat draconian at times, the standards go a long way in ensuring that code at these big tech companies is consistent, well structured and well reviewed. Most of all, it provides guidance to junior engineers who are still figuring out how to write clean, readable code.

Going from Google to Lyft, I observed two issues around code reviews on my team:

1. PR iterations often took over a day to get reviewed
2. Code reviews were shallow — they were either nit-picky or simply rubber stamped without the reviewer doing due diligence

This impacted my team’s velocity as well as code quality. To help mitigate these issues, I came up with some code review standards/guidelines for the team. These guidelines were developed by leveraging suggestions from my teammates, my own personal experience as a software engineer as well as various online resources. Feel free to use some or all of these guidelines for your own teams or projects, or to distribute them freely:

On approvals

• Approve a PR if the change is safe to ship as-is.
• Lean towards granting approval if all of your comments are nits/suggestions for improvements. Not giving an approval implies that the author must change their code in order to get the approval.
• If the change is not safe to ship as-is (e.g. if it may cause an outage/SEV), lock the PR to prevent an accidental merge.

On review timeliness