Code Review Best Practices (Palantir)
(…) This blog post presents Palantir’s take on code reviews. We perform code reviews (CRs) in order to improve code quality and benefit from positive effects on team and company culture. For example:
- Committers are motivated by the notion of a set of reviewers who will look over the change request: the committer tends to clean up loose ends, consolidate TODOs, and generally improve the commit. Recognition of coding expertise through peers is a source of pride for many programmers.
- Sharing knowledge helps development teams in several ways:
* A CR explicitly communicates added/altered/removed functionality to team members who can subsequently build on the work done.
* The committer may use a technique or algorithm that reviewers can learn from. More generally, code reviews help raise the quality bar across the organization.
* Reviewers may possess knowledge about programming techniques or the code base that can help improve or consolidate the change; for example, someone else may be concurrently working on a similar feature or fix.
* Positive interaction and communication strengthens social bonds between team members. - Consistency in a code base makes code easier to read and understand, helps prevent bugs, and facilitates collaboration between regular and migratory developer species.
- Legibility of code fragments is hard to judge for the author whose brain child it is, and easy to judge for a reviewer who does not have the full context. Legible code is more reusable, bug-free, and future-proof.
- Accidental errors (e.g., typos) as well as structural errors (e.g., dead code, logic or algorithm bugs, performance or architecture concerns) are often much easier to spot for critical reviewers with an outside perspective. Studies have found that even short and informal code reviews have significant impact on code quality and bug frequency.
- Compliance and regulatory environments often demand reviews. CRs are a great way to avoid common security traps. If your feature or environment has significant security requirements it will benefit from (and probably require) review by your local security curmudgeons (OWASP’s guide is a good example of the process).
(…)
https://medium.com/palantir/code-review-best-practices-19e02780015f
