This post is an extension
to a talk I gave recently at work.
It was arranged at short notice
and the audience were experienced engineers,
so I kept it brief and tried not to be patronising.
But the feedback afterwards was quite positive
and there were some questions too,
so here's the extended version
for anyone interested.
Note that our team conducts pre-merge reviews,
and some of the suggestions are specific to that context.
I actually prefer post-merge reviews,
but am yet to persuade everyone I work with
that they're better.
Oh, and trigger warning:
this post contains opinions.

Pointer's platform is  dedicated to providing insights and resources for software developers and technology enthusiasts, focusing on programming languages, frameworks, and development tools. Through articles, tutorials, and coding challenges, Pointer offers insights into building software solutions and advancing programming skills. Developers can learn about new programming languages, coding techniques, and best practices in software development.

Pointer

Code review is important for shared ownership and ensuring the big picture is considered. Good code review involves clear descriptions, thorough understanding of the code, reviewing tests, constructive commenting, and withholding approval until full understanding is achieved.

The art of good code review

in these fast meta-word here is the sumirized article with GPT-4 :
The article titled “The Art of Good Code Review” is an extension of a talk given by the author, aimed at experienced engineers, discussing the importance and methodology of effective code review, particularly in the context of pre-merge reviews.
Key Points of the Article:
<ol>
<li>
Purpose of Code Review:
<ul>
<li>Shared Ownership: The primary reason for code review is to establish shared ownership of the code, ensuring multiple people understand, feel responsible for, and are empowered to modify and maintain the code. This approach helps mitigate risks associated with code failure or personnel changes.</li>
<li>The Big Picture: Code reviews serve to maintain a broader perspective on how individual code components fit within the overall system, considering aspects like security, performance, and robustness.</li>
<li>Lesser Reasons: Secondary benefits include improving code quality, adhering to conventions, and identifying bugs, but these are seen as less critical since tools can often handle these aspects.</li>
</ul>
</li>
<li>
Indicators of Poor Code Review:
<ul>
<li>Superficial approvals (e.g., “lgtm” with no deeper engagement).</li>
<li>Excessively rapid reviews suggesting inadequate examination.</li>
<li>Overemphasis on trivial changes while overlooking complex parts.</li>
</ul>
</li>
<li>
Responsibilities of the Code Author:
<ul>
<li>Clear Descriptions: Provide a detailed, clear, and precise description of the change, including context, purpose, and approach.</li>
<li>Minimal and Purposeful Changes: Ensure changes are necessary and avoid unnecessary alterations.</li>
<li>Automated Checks: Complete all automated checks before submitting for review.</li>
<li>Test Coverage: Include tests for new features or behavior modifications.</li>
<li>Self-Review: Conduct an initial review to catch simple errors or omissions.</li>
</ul>
</li>
<li>
Characteristics of Good Code Reviews:
<ul>
<li>Thorough Description Assessment: Ensure the description is comprehensive and understandable.</li>
<li>Detailed Code Examination: Read every part of the code, ask questions for clarification, and build a complete understanding.</li>
<li>Test Review: Evaluate the tests for clarity and effectiveness.</li>
<li>Constructive Commenting: Approach discussions considerately, honestly, and with an open mind.</li>
<li>Appropriate Approval: Understand the change fully before approval, and pre-approve minor corrections.</li>
</ul>
</li>
</ol>
<hr>
<h2>Code Review Checklist:</h2>
<ol>
<li>Security</li>
<li>Observability</li>
<li>Performance</li>
<li>Robustness</li>
<li>Complexity</li>
<li>Readability</li>
<li>Maintainability</li>
<li>Over-engineering (YAGNI)</li>
<li>Using existing abstractions correctly</li>
<li>Naming stuff</li>
<li>Comments</li>
<li>Tests</li>
<li>Doing one thing at a time (separate PRs)</li>
<li>“Do I want to maintain this?”</li>
</ol>
<hr>
<h2>Conclution</h2>
The article emphasizes the importance of understanding, collaboration, and responsibility in the code review process, highlighting that it’s not just about finding errors but also about ensuring the quality and maintainability of the codebase as a whole.