- Community and Accountability — We Become Together
- Collaborative Development Done Well
- Iron Sharpens Iron (Prov 27:17)
- Code Reviews That Build People Up
A code review can be an invitation to learn or an exercise in humiliation. The difference often lies less in the technical comment than in the assumptions behind it.
Why this matters
A good review protects users and maintainers. It checks correctness, security, readability, tests and the fit between the change and the wider architecture.
Practices that shape us
Reviewers should distinguish blockers from preferences, explain the reasoning behind suggestions and ask questions before assuming negligence. Authors should provide context and treat feedback as collaboration rather than a verdict on their competence.
Where things become difficult
Automated formatting and linting remove unnecessary arguments. Small pull requests make thoughtful review possible. Shared standards keep personal taste from becoming an invisible rule.
A better way forward
The goal is not to prove the reviewer is clever. It is to leave the code stronger and both people better equipped for the next change.
Questions to consider
- Where am I relying on individual effort when shared responsibility would be healthier?
- What could I do this week to listen more carefully and make accountability easier?
- Which practice would help this community become more truthful, welcoming and sustainable?
Community grows through habits, not slogans. The next step is to put one of these principles into practice and notice what changes.
Review is part of design
A pull request is often treated as the final obstacle between a developer and deployment. That framing makes review feel adversarial. In reality, review is one stage of design. The author has proposed an implementation; the reviewer tests its assumptions against requirements, operational risks and the wider codebase. A useful review may reveal that the feature works but creates an awkward dependency or hides an error from the user. Those discoveries are easier to address before release than after customers rely on the behaviour.
Give reviewers enough context
A reviewer should not have to reverse-engineer the reason for every change. A concise description can explain the problem, intended behaviour, affected areas, test evidence and known trade-offs. Screenshots help with interface changes; example requests help with APIs. For data migrations, explain rollback and compatibility. Context reduces the temptation to judge unfamiliar code against assumptions that do not apply. It also helps future maintainers understand why the change happened. Good review begins before the first comment is written.
Separate defects from preferences
Not all review comments carry equal weight. A potential data leak is a blocker. A missing test for a critical branch may be a blocker. A suggestion to rename a variable may be helpful but optional. Reviewers should make that distinction explicit. Otherwise the author must guess which comments are mandatory, and discussions about formatting can consume more attention than serious risks. Team conventions can resolve recurring preferences automatically. A reviewer who explains the consequence of a requested change is more useful than one who merely asserts a preferred style.
Ask questions that reveal assumptions
Consider a method that assumes every grave record has a valid location. Rather than writing ‘This is broken’, a reviewer might ask what happens when the coordinates are absent or approximate. The question points to a real domain case without assuming the author overlooked it. Perhaps the author has already handled it elsewhere; perhaps the test suite lacks coverage. Either way, the exchange improves shared understanding. Questions are not always softer than statements, but they are often more precise because they focus attention on behaviour.
Review security and data boundaries
Security concerns deserve deliberate attention. Does a query respect tenant or parish boundaries? Can a user modify a record outside their assigned scope? Are imported values validated before database operations? Does an error message expose sensitive information? Reviewers should think about authorisation as well as authentication. A user who is logged in is not necessarily permitted to perform every operation. These checks are particularly important in administrative systems where a simple interface may conceal complex relationships and permissions.
Tests are a conversation about intent
A test is not only a machine-executable check. It communicates which behaviour the team considers important. During review, ask whether the tests demonstrate the feature’s contract rather than merely mirror the implementation. Consider success paths, missing values, invalid input and permission boundaries. A regression test for a discovered defect protects future changes. But coverage percentages should not become a substitute for thought. A small number of meaningful tests may offer more confidence than many tests that never exercise a failure.
Avoid overwhelming the author
A long list of loosely related suggestions can make review feel impossible to finish. Group comments by importance and avoid repeating the same point on several lines. Where a change requires substantial redesign, consider a conversation rather than dozens of fragmented remarks. If the reviewer sees a broader architectural concern, explain whether it must be addressed now or can be recorded separately. Respect the author’s time and attention. Review should improve the change, not demonstrate how many observations the reviewer can make.
Authors have responsibilities too
A constructive review is a two-way process. Authors should avoid treating every question as an attack, respond with evidence and acknowledge mistakes openly. They should not bury a controversial change among unrelated edits or dismiss concerns because the code passes locally. If they disagree with a suggestion, they can explain the trade-off respectfully. Sometimes the right response is to revise the code; sometimes it is to document why the original approach is appropriate. The shared objective remains a reliable outcome.
Design for asynchronous teams
Not everyone works at the same time. A thoughtful review process supports colleagues across schedules and time zones. Agree expectations for response times, keep comments understandable without a live meeting and avoid leaving urgent work dependent on a single unavailable reviewer. When a discussion moves to a call, record the decision in the pull request afterwards. This prevents important context disappearing into private conversation. Asynchronous work can be inclusive and efficient when written communication is treated as engineering work.
Automation should handle repetition
Formatters, linters, static analysis and automated tests can catch many predictable issues before a human begins reviewing. That frees people to consider correctness, maintainability and user impact. Automated tools are not infallible, and their findings still require judgement. But arguing repeatedly about indentation or import order wastes scarce attention. Agree standards, automate them and focus human review on questions that require understanding. This also reduces the influence of individual taste and makes expectations clearer to newcomers.
A review culture that teaches
Experienced developers have an opportunity to explain not only what should change but why. A concise explanation of database transactions, error handling or API boundaries can become a lesson the author applies for years. Newer developers also bring fresh eyes and can identify confusing assumptions that veterans no longer notice. Pairing people across experience levels should not mean that only one person is allowed to ask questions. A good review culture treats curiosity as a strength and acknowledges useful discoveries regardless of seniority.
Review the process itself
If reviews routinely take days, important changes may accumulate in large batches. If approvals happen instantly, scrutiny may be superficial. If the same person must approve everything, the team has a bottleneck. Periodically examine review turnaround, recurring defects and whether comments are constructive. Ask contributors what makes reviews difficult. Improve the workflow without turning metrics into quotas. The ultimate measure is whether the code becomes safer and the team becomes more capable. That is what building people up should mean.
An example of a review that matters
Imagine a developer adding an administrative search feature. The interface works, the tests pass and the results appear quickly. During review, a colleague asks whether the query restricts results to records the current user is authorised to see. The question reveals a problem: the developer tested the search as an administrator and did not exercise a restricted account. A constructive response is to add the missing authorisation condition and write tests for both permitted and forbidden access. The reviewer has protected users without suggesting the author is incompetent. The author has improved the system by taking the concern seriously. That exchange is more valuable than a dozen comments about variable names.
What to do when a review stalls
A pull request can remain open because nobody feels authorised to make the final decision. Reviewers offer contradictory preferences, the author revises the same section repeatedly, and the original problem is forgotten. Teams can prevent this by identifying the responsible approver, distinguishing essential requirements from optional suggestions and setting a reasonable time for discussion. If a design question is too large for a comment thread, record the alternatives and arrange a focused conversation. Afterwards, write the decision back into the review so colleagues can understand it. The goal is not endless agreement. It is a decision that is transparent, defensible and safe to maintain.
A checklist for the next review
Before requesting approval, describe the intended behaviour and include evidence of testing. As a reviewer, begin with the highest-risk questions: permissions, data integrity, error handling and compatibility. Consider what happens when input is missing or unexpectedly large. Ask whether the change can be rolled back safely. Then look at clarity and maintainability. Finish by recognising what the author has done well. A review should leave a useful record of reasoning, not simply a green tick. When this becomes routine, teams can release with greater confidence and newer colleagues learn how experienced developers approach difficult decisions.