TL;DR
- In a team of 3 to 5 developers, the default should be 1 mandatory approval, green CI and small PRs. 2 approvals almost always block delivery.
- A good code review does not try to find every bug. It tries to protect architecture, readability, security, data migrations and external contracts.
- PR size is the metric that most changes the outcome. Above 300 to 500 changed lines, review quality drops sharply.
- Define an explicit SLA: first response in under 4 working hours, full review in under 24 working hours. Without this, reviews become invisible queues.
- Automate formatting, linting, tests and obvious checks. Humans should review decisions, not spaces, file names or imports.
The real objective of code review in a small team
In a team of 3 to 5 developers, code review is not a ceremony to pretend there is rigour. It is a risk control mechanism. The problem is that many teams copy processes from companies with 200 engineers and end up with a slow, frustrating and not very useful version.
The objective should be clear: increase quality without blocking cadence.
For me, a small and well executed code review should answer five questions:
- Does this do what the ticket says?
- Does this fit the existing architecture?
- Does this introduce risk in data, security, performance or external contracts?
- Will this be easy to maintain in 6 months?
- Are there enough tests for the risk of the change?
Notice what is not on the list: personal style preferences, endless discussions about names, unrelated refactors, manual formatting. That should be automated or decided beforehand.
In small teams, the cost of context is enormous. If you have 4 developers and 2 are blocked on reviews for half a day, you have lost 25% of the team’s productive capacity during that period. This sounds exaggerated until you measure the time between opening a Pull Request and merging it.
The metric I like to track is simple: p95 of time to merge. For a healthy product team, normal changes should have a p95 below 48 working hours. Critical bugs and small changes should be below 24 working hours. If a PR sits waiting for 4 days, you do not have a quality process. You have stalled inventory.
The 300 LOC rule and why large PRs are a lie
The biggest mistake in code reviews is accepting huge PRs as normal.
From 300 to 500 changed lines onwards, excluding generated lockfiles and snapshots, the review starts to degrade. The reviewer stops understanding the full intent, starts visually scanning and begins approving out of fatigue. Nobody likes to admit this, but it happens.
The practical rule I recommend:
| Type of change | Healthy size | Approval | Expected time |
|---|---|---|---|
| Small bug | up to 100 LOC | 1 reviewer | same day |
| Normal feature | 100 to 300 LOC | 1 reviewer | under 24 working hours |
| Data migration | any size | 1 senior reviewer | careful review |
| Cross-cutting refactor | 300 to 800 LOC | split into phases | do not review everything at once |
| PR above 800 LOC | suspicious | justify | usually split |
LOC is not a perfect measure. A 40 line change in a SQL query can be more dangerous than 400 lines of UI. Even so, it is an excellent proxy for cognitive load.
The point is not to create a bureaucratic rule. It is to force the right question: why can this not be split?
A pattern that works well:
- PR 1: prepare structure without changing behaviour.
- PR 2: add new behaviour behind a feature flag or isolated path.
- PR 3: connect behaviour in the UI or API.
- PR 4: remove old code.
This makes review easier and reduces risk. It also makes rollback easier. If everything goes into a single PR with database, API, frontend and clean-up, any problem forces you to revert a large package.
There is one exception: generated code. For example, updating an SDK, regenerating OpenAPI types or snapshots. In that case, the PR may have many lines, but the reviewer should focus on the semantic diff: schema, contract, removed endpoints, breaking changes. They should not pretend they read every generated line.
Approvals: 1 is almost always better than 2
In teams of 3 to 5 developers, requiring 2 approvals by default is usually a mistake.
With 3 developers, 2 approvals mean the whole team reviews almost everything. With holidays, illness, support focus or meetings, the flow stops. The quality gain rarely offsets the cost.
My recommendation:
- 1 mandatory approval for normal changes.
- 2 approvals only for critical areas.
- Auto-merge allowed when CI is green and the approval exists.
- Explicit technical owner for sensitive parts of the system.
Critical areas include authentication, permissions, payments, billing, destructive migrations, clinical data, ERP integrations, stock synchronisation, webhooks and infrastructure changes.
You can configure this with CODEOWNERS in GitHub or GitLab. The official documentation is worth reading, especially GitHub branch protection rules and GitLab 17 merge request approvals.
Simple CODEOWNERS example:
# Autenticação e permissões
/apps/api/src/auth/ @ana @rui
# Pagamentos e webhooks
/apps/api/src/billing/ @rui
# Infraestrutura
/infra/ @miguel
/.github/workflows/ @miguel
This does not solve everything, but it avoids a common trap: the person who knows the area least approving the most dangerous change because they were the first to see the PR.
Practical comparison:
| Model | Pros | Cons | When to use |
|---|---|---|---|
| 1 approval | Fast, suitable for small teams, fewer queues | Less human redundancy | Default for product |
| 2 approvals | More eyes on high risk, useful for compliance | Slow, creates blockers, dilutes responsibility | Payments, security, critical data |
| No approval with pair programming | Fast flow, immediate feedback | Requires discipline and records | Spikes, hotfixes, real pair work |
A strong opinion: if two people genuinely did pair programming during implementation, it makes no sense to require a third person to review everything as routine. In that case, the PR should say “done in a pair with X” and pass if CI is green, except in critical areas.
What should be automatic and what should be human
If a reviewer comments “missing semicolon”, “sort the imports” or “Prettier changed this”, the process is badly designed.
The minimum acceptable in 2026 for a small team:
- Automatic formatting: Prettier 3.x, Black 24.x, gofmt, rustfmt, depending on the stack.
- Linting with agreed rules: ESLint 9, Ruff, golangci-lint.
- Type checking: TypeScript 5.x with
strict, mypy or equivalent when it makes sense. - Relevant unit and integration tests.
- Full build.
- Basic security checks: npm audit with clear criteria, OSV Scanner or Dependabot.
- Branch protection to prevent merging with red CI.
For Node.js 22 LTS and TypeScript, I would not let a PR pass without typecheck, lint and tests. The same applies to migrations: if you use Prisma, Drizzle, Flyway or Liquibase, the pipeline should validate that the migrations apply to a clean database.
The human reviewer should focus on this:
- Does the change preserve domain invariants?
- Are error cases handled?
- Is there idempotency where it is needed?
- Is the migration compatible with deployment without downtime?
- Has the API contract changed?
- Are there side effects in queues, webhooks or jobs?
- Do the tests cover the real risk?
A classic example: payment webhooks. The linter will not understand whether you process the same event twice. An experienced reviewer should ask: “is this idempotent by event_id?” For Stripe, for example, the official documentation recommends verifying webhook signatures and handling repeated events. This is not a detail. It is production.
PR template that forces thinking before the review
A good template reduces lazy reviews. It does not need to be long. In fact, if it is long, nobody fills it in properly.
I would use something like this:
## O que mudou
Resumo curto da alteração.
## Como testar
Passos concretos ou comando executado.
## Risco
Baixo, médio ou alto. Explicar porquê.
## Dados e migrations
Há alteração de schema ou dados? Sim ou não.
## Contratos externos
Altera API, webhook, evento, fila ou integração? Sim ou não.
## Screenshots ou evidência
Quando aplicável.
This changes the conversation. Instead of the reviewer trying to guess everything from the diff, the author makes intent and risk explicit.
The “How to test” section is particularly important. “Tested locally” is not enough. I prefer something concrete:
- “I ran
npm test -- invoices.service.spec.ts.” - “I validated subscription creation and cancellation in the staging environment.”
- “The migration was applied to an anonymised copy with 8GB.”
- “I did not test on mobile, only desktop.”
The last sentence sounds bad, but it is good. Operational honesty is better than false confidence.
Another useful practice: risk labels.
risk:low: small UI, copy, isolated change.risk:medium: business logic, endpoints, queries.risk:high: data, auth, payments, infra, migrations.
With this, the team stops treating all PRs as equal. A copy change in the frontend should not have the same process as a permissions change.
Daily flow: SLAs, ownership and blockers
Without an SLA, code review becomes invisible debt. Everyone “will look at it shortly” and nobody does.
For a team of 3 to 5 developers, I recommend these rules:
- PR opened by 15:00 should receive a first response on the same day.
- Small PR should be reviewed in under 4 working hours.
- Normal PR should be reviewed in under 24 working hours.
- PR stalled for more than 24 hours appears in the standup.
- PR with no response for more than 48 hours can be escalated without drama.
This does not mean interrupting deep work every 10 minutes. It means creating windows. For example, each developer reviews PRs at 11:30 and 16:30. Two windows a day are enough for small teams.
I also like a simple rule: whoever opens the PR should choose the reviewer. Do not throw it to “the team”. Diffuse responsibility is a factory for delays.
If there is strong specialisation, use controlled rotation:
| Day | Primary reviewer |
|---|---|
| Monday | Developer A |
| Tuesday | Developer B |
| Wednesday | Developer C |
| Thursday | Developer A |
| Friday | Developer B |
This works when there are many small PRs. If the team has few PRs, choosing the reviewer by context is better.
The standup should include only three questions about PRs:
- Which PRs are blocked?
- Which PRs need a technical decision?
- Which PRs are too large and should be split?
Do not turn the standup into a Jira reading session. The objective is flow.
Common pitfalls that show up in production
The first pitfall is reviewing too late. If the architectural decision is wrong, discovering that when the PR has 900 lines is expensive. For uncertain work, review before the code: a short technical note, a diagram, or a 2 hour spike.
The second is confusing “comment” with “mandatory request”. Use explicit language:
- “blocking”: must change before merge.
- “non-blocking”: suggestion, can be left for later.
- “question”: I need to understand better.
This reduces friction. Without this distinction, authors interpret everything as mandatory criticism and reviewers think they have been ignored.
The third is discussing style in the PR. Style should be in short guides and tools. If the team is still discussing folder names in every PR, a baseline decision is missing.
The fourth is approving without running the application. Manual testing is not always necessary, but for visual changes, payment flows, permissions or migrations, just looking at the diff is insufficient. A reviewer should ask for evidence: screenshot, short video, staging logs, test output, rollback plan.
The fifth is doing squash or rebase without understanding the impact. Squash merge is good for a clean history, but it can hide commits that documented important phases of a migration. Merge commits preserve context, but clutter history in teams that do not discipline branches. Rebase leaves a linear history, but can be dangerous if the team is not comfortable with conflicts.
Honest comparison:
| Strategy | Pros | Cons |
|---|---|---|
| Squash merge | Clean history, easy to revert the whole PR | Loses commit granularity |
| Merge commit | Preserves full history | Noisy history |
| Rebase merge | Linear and readable | Requires care with conflicts |
For small product teams, I would choose squash merge by default, with well written PR titles. Exception: complex migrations or phased work where commit granularity helps auditing.
Final checklist to start this week
You do not need a heavy process. You need small rules, applied consistently.
I would start with this:
- Enable branch protection on the main branch.
- Require green CI before merge.
- Require 1 approval by default.
- Create CODEOWNERS for critical areas.
- Adopt a short PR template.
- Define a first response SLA.
- Measure time to merge for 2 weeks.
- Split PRs above 500 LOC, unless justified.
- Mark comments as blocking or non-blocking.
- Do early review for large technical decisions.
The tools can be GitHub, GitLab or Bitbucket. The principle is the same. GitHub Actions, GitLab CI or CircleCI also work, as long as the pipeline does not take long. If the pipeline takes 35 minutes, developers will try to work around it. Aim for under 10 minutes on the main checks. Long tests can run nightly or in specific phases, but the main gate must give fast feedback.
Conclusion: code review in a small team should be lightweight, explicit and focused on risk. One well done approval, decent CI and small PRs beat heavy processes almost every time. The worst system is the one that looks rigorous, but everyone works around because it causes delays.
If you are facing a similar problem, book a conversation at https://impact-origin.com/agendamento.