Jan Mikoláš
Glossary
Code review
Code review is collaboration between an author and reviewer around a specific change. Automation finds repeatable mistakes; people check whether a change makes sense in the product, domain, and operation.
Short definition
Before a merge, review checks not only code but also the decisions within it.
Code review does not mean finding typos or competing over who knows more shortcuts. The author explains the problem, chosen approach, risk, and verification. The reviewer reads the changed diff, surrounding code, and possibly application behaviour, so they can provide useful feedback before the pull request is merged.
CI, static analysis, and automated tests are important gates. They verify rules that can be described and repeated. They do not automatically know business intent, whether a solution is understandable to the next developer, whether a data-flow change is appropriate, or whether a new feature breaks an important customer scenario.
The term is sometimes called a code-change review. It is broader than a comment on one line: good review includes the change description, diff scope, tests, security, and operational impact. It is not, however, a formal audit that guarantees an entire application is error-free or secure.
What it is for
Shared understanding of a change before it becomes part of a product
Good review has a proportionate scope and a clear goal. A reviewer is not a passive approval stamp, and an author does not need to accept every suggestion without discussion.
- checking that a change matches the request, chosen solution, and system boundaries
- finding errors in edge cases that a test does not cover or cannot express easily
- reviewing permissions, sensitive data, failures, retries, and operational impact
- making names, class responsibilities, and public contracts clear together
- sharing knowledge of a system area so it is not held by only one author
- recording decisions and the reason for an exception for future maintenance
Practical example
A payment webhook looks short but carries important questions
A webhook after a payment must do more than change an order state. The reviewer asks whether its signature is verified, whether the order belongs to the correct context, and what happens on duplicate delivery. Idempotence is a business requirement here that linting or a green build cannot automatically confirm.
A useful comment is concrete and explains the risk: “Can the provider send the same event again? If it can, we need to store its ID before changing the state, or we will send two invoices.” It does not only say “this is wrong”; it lets the author check the assumption and find an appropriate solution.
PHP
public function paymentWebhook(Request $request): Response
{
$event = json_decode($request->getContent(), true);
$order = $orders->get($event['orderId']);
$order->markPaid();
$orders->save($order);
return new Response('', 204);
}How review works
From understandable intent to a deliberate merge decision
Text diagram: author → description and diff → automated checks → reviewer → comments or approval → corrected change → merge.
- The author prepares a change They split work into a readable diff and describe the problem, solution, tests, limits, and any release plan.
- Automation gives fast feedback CI runs linting, tests, and static analysis. A failure is input for a fix, not work for a reviewer to rewrite manually.
- The reviewer reads context first They compare the request, description, and diff. Only then do they assess individual lines, naming, and implementation detail.
- Comments lead to a decision A comment can be a question, suggestion, or blocking problem. The author responds, fixes code, or documents with the reviewer why a solution is proportionate.
- Approval or requested changes Approval says the reviewer sees no obstacle to merging within the given scope. Requested changes marks a problem for the author to resolve and have checked again under the team rules.
What review actually checks
The diff, context, and risk matter more than the number of comments.
Not every observation has the same severity. A team should distinguish a blocking problem from an optional suggestion or a question for understanding.
Diff and change scope
A reviewer checks that the diff solves the described problem and does not add unrelated refactoring. A small logical unit is easier to understand, test, and revert than one large mixture of changes.
Author and reviewer
The author is responsible for an understandable proposal and response to feedback. The reviewer is responsible for careful review in a proportionate time, not for taking over implementation or ownership of the entire change.
Comment, approval, and requested changes
A comment may only share an observation. Approval is not a global quality certificate. Requested changes should be clear, factual, and tied to a risk; its effect on mergeability is determined by platform settings and protected branches.
Automation and human judgement
PHPStan, tests, and CI expose repeatable contract violations. A reviewer assesses, for example, unclear data ownership, a wrongly chosen transaction boundary, a missing fallback, or an unsuitable API for a client.
Security and permissions
Authentication determines who signed in; authorisation determines what they may do. Review checks both boundaries, secret handling, untrusted-input validation, and whether logs contain sensitive data.
Business and operational relevance
A change to an order status, price, or stock can be technically clean and still wrong. A reviewer checks assumptions, concurrency, failures, metrics, data migration, and the impact of a partial failure.
Benefits and limits
A second set of eyes helps, but does not replace responsibility or testing.
Benefits
- an error or unclear assumption can surface before merge
- shared knowledge of code and domain reduces dependency on one author
- more consistent contracts, names, and component boundaries
- a traceable decision for a risky or unintuitive change
Common mistakes
- approving without reading the diff because CI is green
- focusing review on personal style and missing business or security risk
- submitting a diff that is too large or unrelated for anyone to read carefully
- leaving a blocking comment without an explanation, priority, or follow-up check
- treating approval as a production deploy or a guarantee that no error can occur
When it fits
For every change whose impact deserves a second understanding.
Review pays off from a small fix to a critical payment change. Its depth should match risk: a typo does not need the same process as an order migration, permission change, or payment-provider integration. An urgent fix may have a shorter process, but it still needs a traceable reason and later verification.
Review is not the only way to coordinate a large, unclear task. First clarify the request, architecture, or experiment in a smaller step. A pull request then shows an implementable change; it does not replace every product discussion.
Practical rules
Prepare review so it helps quickly and factually.
High-quality review starts before the request for review is sent.
- the author states the problem, solution, tests, limits, data risk, and any rollout or rollback
- keep the diff small, logically cohesive, and free of unrelated formatting
- the reviewer reads the description and main flow first, then individual-line details
- for a blocking observation, describe the impact, expected behaviour, or question that needs resolution
- after code changes, rerun relevant CI and request re-review when the team rule requires it
- do not bypass required approval or branch protection without a traceable reason and accountability
Frequently asked questions
Review without needless confusion
Does code review replace CI and static analysis?
No. CI, tests, and static analysis find repeatable mistakes. Code review adds human understanding of intent, context, risk, and business impact.
Is approval a guarantee that code has no error?
No. It is a reviewer decision over a particular change scope and available context. Tests, monitoring, and author responsibility are still needed.
Does every comment have to be fixed?
Not necessarily. The author and reviewer should reach a clear decision. A blocking problem is fixed or deliberately escalated; a recommendation can be deferred when its reason is recorded and the team agrees.
Should the reviewer rewrite the change for the author?
Usually not. A reviewer should explain the risk, question, or direction for improvement. Taking over implementation only fits exceptional cases, such as pairing or an urgent incident.
How I work with quality
I connect review with tests, static analysis, and clear ownership of a change.
Quality emerges in everyday workflow: a small diff, automated checks, factual feedback, and verification of impact after merge.