Code Review Checklist for Small Teams: Quality, Security, Maintainability

Code review fails when it becomes a ritual:

open pull request
skim diff
comment on naming
approve

A useful review asks a more important question:

What could go wrong
because of this change?

That includes:

incorrect behavior

security weakness

data corruption

broken compatibility

poor maintainability

performance regression

deployment surprise

A small team cannot spend hours reviewing every five-line change, so the objective is not maximum ceremony.

The objective is:

apply reviewer attention
where the risk is highest

1. Review risk, not just code style

Small-team code review workflow (diagram)

Human review is expensive.

Use automation for things machines are good at:

formatting

linting

type checking

unit tests

static analysis

dependency scanning

build verification

so humans can focus on:

intent

behavior

risk

architecture

security

operations

Review priorities

A useful order is:

1. Is the change necessary and correctly scoped?

2. Is the behavior correct?

3. Is it safe?

4. Are failure paths handled?

5. Are tests meaningful?

6. Will the code remain understandable?

7. Can it be deployed and rolled back safely?

Do not optimize for comment count

Ten comments about punctuation are not better than one comment that catches:

missing authorization check

Approval means acceptable risk

It does not mean:

I would have written
every line the same way

2. Check intent and pull request scope first

Before reviewing code, understand:

What problem is being solved?

What should change?

What should not change?

How will users notice?

How will operators notice?

Good pull request description

Problem:
Expired invitations can still be accepted.

Change:
Reject invitations after expiresAt.

Behavior:
Return 410 for expired invitations.

Tests:
Added before / at / after expiry cases.

Deployment:
No migration required.

Scope creep is a review problem

A bug fix should not casually include:

framework upgrade

database redesign

unrelated formatting

renaming 50 files

unless those changes are necessary.

Ask whether the diff is reviewable

A large change may be correct but difficult to verify.

Consider splitting:

mechanical rename

schema migration

behavior change

cleanup

into independently understandable pieces.

Check generated files separately

If a diff includes:

generated lock file

compiled bundle

generated client

focus on the source change responsible for it while still checking for unexpected dependency or artifact changes.

3. Verify correctness and edge cases

Start with the happy path:

Does the intended user action work?

Then immediately look at boundaries.

Common edge cases

null

missing value

empty collection

zero

negative value

maximum size

duplicate input

already deleted item

expired state

concurrent update

Check state transitions

Suppose:

Pending -> Approved

is valid.

Ask whether:

Cancelled -> Approved

is accidentally possible.

Check failure behavior

If a database operation fails halfway through:

what has already changed?

Check return semantics

Review changes from:

null

to:

empty object

or:

false

to:

exception

because callers may depend on the old contract.

Look for hidden behavior changes

changed ordering

changed default

changed timeout

changed rounding

changed timezone

changed case sensitivity

4. Review the tests as carefully as the implementation

A passing test suite does not prove the new behavior is tested.

Ask what new behavior the test proves

Weak:

assert response != null

Stronger:

given expired invitation

when accept is requested

then status is 410

and membership is not created

Review positive and negative cases

valid input succeeds

invalid input fails

unauthorized caller fails

dependency failure is handled

no unwanted side effect occurs

Check boundary tests

For:

expiresAt

test:

before expiry

exactly at expiry

after expiry

Tests should not simply reproduce implementation

If production code calculates:

x * 1.2

and the test calculates:

expected = x * 1.2

using the same logic, both may contain the same mistake.

Avoid brittle implementation tests

Prefer:

observable result

over:

private method called
exactly three times

unless the interaction itself is important.

Check regression protection

A bug fix should usually include a test that would fail:

before the fix

and pass:

after the fix

5. Perform a focused security pass

Code review quality and security layers (diagram)

Authentication

Does the code correctly identify the caller?

Authorization

More importantly:

May this caller perform
this action on this resource?

Do not confuse:

authenticated

with:

authorized

Input validation

Treat:

HTTP input
message payload
file
webhook
CLI input
third-party response

as untrusted until validated.

Injection

Look for dynamic construction of:

SQL

shell command

HTML

LDAP query

path

template expression

from untrusted input.

Parameterized database access

Prefer:

query parameters

over:

string concatenation

Shell execution

Prefer process APIs with separate arguments rather than:

"command " + userInput

Sensitive logging

Check whether new logs contain:

password

token

session ID

API key

payment data

personal information

Secrets

Reject committed:

credentials

private keys

production tokens

Dependency changes

A one-line import can introduce:

new transitive dependencies

new network behavior

new license obligations

new supply-chain risk

Review why the dependency is needed.

6. Treat data and migrations as high-risk changes

Application code can be rolled back.

Corrupted production data may be much harder to repair.

Review schema migrations explicitly

Ask:

Is migration backward compatible?

Will old application instances still work?

Does it lock a large table?

Does it rewrite every row?

Can it be retried?

Can it be rolled back?

Expand before contract

Safer migration pattern:

add new field

deploy compatible code

backfill

switch reads

stop old writes

remove old field later

Check data constraints

Application validation:

email must be unique

may not be enough under concurrency.

A database uniqueness constraint may still be required.

Review transactions

If code changes:

transaction start

commit

rollback

inspect partial-failure behavior carefully.

Backfills need operational review

A script that updates:

40 million rows

deserves review for:

batching

retry

progress

load

idempotency

restart behavior

7. Review for maintainability, not personal taste

Avoid:

I prefer this style

when both approaches are clear and consistent.

Review naming

Names should communicate:

domain meaning

ownership

side effects

units

Compare:

process()

with:

reserveInventory()

Review function responsibility

A function that:

validates request

calculates price

writes database

sends email

formats response

may be difficult to test and change.

Review abstraction level

Watch for:

giant functions

deep call chains

unnecessary interfaces

wrapper around wrapper

premature generic framework

Duplication is contextual

Two similar lines do not automatically require abstraction.

Ask:

Will these behaviors
change for the same reason?

Prefer explicit over clever

Code that saves:

four lines

but takes:

five minutes to understand

may be a poor maintenance tradeoff.

8. Look for expensive operations where scale matters

Not every pull request needs a performance investigation.

Look deeper when changes touch:

large datasets

hot request path

database queries

nested loops

serialization

image processing

network fan-out

Watch for N+1 queries

load users

for each user:
    load orders

can produce:

1 + N queries

Check repeated scans

for each item:
    list.contains(item)

can hide quadratic work when:

contains = O(n)

Review allocation

Hot loops creating:

large temporary lists

strings

objects

may increase memory pressure.

Review network fan-out

One request that now calls:

30 downstream services

has different latency and failure characteristics.

Ask for measurement when risk is real

Useful evidence:

benchmark

query plan

load test

before / after profile

9. Review concurrency and ordering assumptions

Concurrency bugs often look correct in a local single-request test.

Check read-modify-write sequences

read balance

if balance sufficient:
    subtract amount

write balance

Two concurrent requests may both observe the same starting state.

Ask what protects the invariant

database lock?

atomic update?

transaction?

optimistic version?

distributed lock?

Check ordering

If code emits:

database update

event

ask whether:

event before commit

and:

event after commit

have different failure behavior.

Retries can duplicate side effects

A retryable operation should consider:

idempotency

Async tasks need ownership

Review:

fire-and-forget task

detached future

background thread

for:

error handling

shutdown

cancellation

resource lifetime

10. Protect public contracts and compatibility

Public contracts include more than HTTP APIs.

API response

event schema

database schema

CLI output

config file

library method

file format

Check field removal

Removing:

user_name

may break unknown consumers.

Check type changes

"count": 5

changing to:

"count": "5"

is a breaking contract even if humans see the same value.

Check semantics

Keeping the field name but changing:

milliseconds

to:

seconds

is also breaking.

Review deprecations

Prefer:

add new behavior

support old behavior

migrate callers

remove later

when compatibility matters.

11. Check logs, metrics, timeouts, and failure visibility

Ask:

If this breaks in production,
how will we know?

Logs

Useful logs can include:

request ID

operation

resource ID

safe failure reason

without including secrets.

Metrics

New critical workflows may need:

success count

failure count

latency

retry count

queue depth

Timeouts

A new network call without a timeout can turn:

remote slowness

into:

resource exhaustion

Retries

Review:

which failures are retryable?

how many attempts?

what backoff?

is operation idempotent?

Alerts

Not every feature needs a new alert.

But a new critical dependency should have an operational owner and a detection strategy.

12. Review deployment and rollback safety

A change can pass every test and still be risky to release.

Mixed-version compatibility

During rolling deployment:

old application instance

and

new application instance

may run simultaneously.

Review:

database compatibility

event compatibility

cache format

shared state

Feature flags

High-risk behavior can be deployed disabled:

deploy code

enable for internal users

enable 10%

monitor

increase rollout

Rollback

Ask:

Can we deploy the previous version?

Will the new database schema still work?

Did we already write irreversible data?

Can the feature be disabled?

Deployment order

Multi-service changes may require:

producer first

or

consumer first

depending on compatibility.

13. Write review comments that are actionable

Weak comment

This is bad.

Better

This endpoint checks authentication
but not ownership of projectId.

A user could request another
user's project if they know the ID.

Could we add the project ownership
check before returning the record?

Separate blockers from suggestions

Useful labels:

blocking

suggestion

question

nit

Explain why

Instead of:

Use a Set.

say:

This membership check is inside
the loop, so the current list scan
makes the workflow quadratic.

Could we build a Set once?

Do not use review to demonstrate superiority

The objective is:

better code
and
shared understanding

Move long design debates out of inline comments

A ten-message thread about architecture may be better handled with:

short synchronous discussion

design note

follow-up issue

14. Scale review depth with change risk

Risk-based code review decision flow (diagram)

Low-risk example

documentation typo

copy change

isolated test cleanup

Normal review may be enough.

Medium-risk example

new endpoint

new business validation

moderate refactor

background job change

Require:

behavior tests

failure review

compatibility review

High-risk example

authentication

authorization

payments

schema migration

data deletion

concurrency

encryption

secret handling

new infrastructure dependency

Consider:

specialist review

security review

migration plan

staged rollout

rollback verification

production monitoring

Risk depends on blast radius

A five-line change to:

global authorization middleware

may deserve more review than:

500-line internal admin report

Ask what happens if the reviewer is wrong

minor visual bug?

failed request?

security exposure?

money loss?

data corruption?

That answer should influence review depth.

15. Copy/paste code review checklist

Small-team code review checklist

Pull request intent
- Is the problem clearly described?
- Is expected behavior clear?
- Is non-goal behavior clear?
- Is the change actually necessary?
- Does implementation match the stated goal?
- Are important product assumptions documented?

Scope
- Is the pull request focused?
- Are unrelated changes included?
- Could risky independent parts be separated?
- Are formatting-only changes mixed with behavior changes?
- Are generated files separated conceptually from source changes?
- Is the diff small enough to understand?

Risk classification
- Is this authentication-related?
- Is this authorization-related?
- Does it handle money?
- Does it delete data?
- Does it modify database schema?
- Does it change public API?
- Does it change event schema?
- Does it affect concurrency?
- Does it add a dependency?
- Does it modify secret handling?
- Does it affect critical infrastructure?

Correctness
- Does happy path work?
- Are boundary cases correct?
- Are missing values handled?
- Are empty values handled?
- Are duplicates handled?
- Are invalid states rejected?
- Are defaults correct?
- Are units correct?
- Are timezones correct?
- Is ordering correct?

Conditions
- Are boolean expressions correct?
- Are operator precedence assumptions obvious?
- Are off-by-one errors possible?
- Are inclusive / exclusive boundaries correct?
- Are null checks correct?
- Are early returns safe?
- Are all branches reachable as intended?

State transitions
- Is current state validated?
- Is requested transition allowed?
- Can terminal state transition incorrectly?
- Can repeated request change result?
- Is idempotency needed?
- Is transition atomic where required?

Errors
- Are expected failures represented correctly?
- Are unexpected failures visible?
- Is original cause preserved?
- Are errors swallowed?
- Are stack traces exposed to users?
- Are safe messages returned?
- Is cleanup executed?
- Does failure leave partial state?

Tests
- Does new behavior have tests?
- Would tests fail before implementation?
- Are happy-path tests present?
- Are negative tests present?
- Are edge cases present?
- Are boundary values tested?
- Are regression tests included for bug fixes?
- Are failure paths tested?
- Are side effects tested?
- Are tests deterministic?

Test quality
- Are tests asserting behavior rather than implementation?
- Are mocks necessary?
- Are mocks too tightly coupled?
- Are fixtures readable?
- Are test names descriptive?
- Are tests independent?
- Are time and randomness controlled?
- Are flaky assumptions introduced?

Coverage
- Do not review coverage percentage alone.
- Check risky branches.
- Check critical contracts.
- Check authorization failures.
- Check transaction failures.
- Check migration behavior.
- Check bugs permanently.

Authentication
- Is identity established from trusted source?
- Are unauthenticated requests rejected?
- Are authentication errors safe?
- Is session / token verification correct?
- Are expiration rules correct?
- Are revocation rules considered?

Authorization
- Is action authorized?
- Is resource ownership checked?
- Is tenant boundary enforced?
- Is role check sufficient?
- Is authorization performed server-side?
- Can client-controlled fields grant privilege?
- Are admin paths protected separately?
- Is default-deny behavior preserved where appropriate?

Input validation
- Are external inputs validated?
- Are types validated?
- Are sizes bounded?
- Are enum values checked?
- Are IDs validated?
- Are nested structures validated?
- Are unexpected fields handled intentionally?
- Are file uploads constrained?
- Are URLs constrained where necessary?

Injection
- Is SQL parameterized?
- Is shell execution safe?
- Is HTML output encoded?
- Is template input safe?
- Are paths constructed safely?
- Are LDAP / query expressions parameterized where supported?
- Is user input used inside regex safely?
- Is eval avoided?

SQL
- Avoid string concatenation with user input.
- Use prepared statements.
- Check query scope.
- Check tenant filter.
- Check authorization filter.
- Check transaction boundaries.
- Check lock behavior.
- Check N+1 queries.

Shell commands
- Prefer direct process APIs.
- Keep executable and arguments separate.
- Avoid shell interpolation.
- Validate file paths.
- Avoid untrusted command fragments.
- Handle exit status.
- Handle timeout.
- Handle cancellation.

Files
- Validate paths.
- Prevent path traversal where relevant.
- Use safe temporary files.
- Check permissions.
- Avoid unsafe overwrite.
- Clean temporary files.
- Handle symbolic links according to threat model.
- Avoid leaking sensitive paths.

Secrets
- No hard-coded credentials.
- No production tokens.
- No private keys.
- No secret values in tests committed publicly.
- Do not log secrets.
- Do not return secrets in errors.
- Redact diagnostic output.
- Use approved secret source.

Sensitive data
- Minimize collection.
- Minimize logging.
- Validate access control.
- Check retention.
- Check encryption requirements.
- Avoid exposing data in URLs where inappropriate.
- Avoid accidental analytics collection.

Dependencies
- Is new dependency necessary?
- Is library maintained?
- Is version pinned appropriately?
- Are transitive dependencies understood?
- Does lock file change as expected?
- Does dependency execute install scripts?
- Does it add network behavior?
- Does it introduce licensing concerns?
- Is standard library sufficient?

Serialization
- Is untrusted deserialization safe?
- Are allowed types constrained?
- Are unknown fields handled?
- Are numeric assumptions safe?
- Are schema versions compatible?
- Are secrets excluded?
- Are defaults intentional?

Database schema
- Is migration backward compatible?
- Can old and new app versions coexist?
- Does migration lock large tables?
- Does migration rewrite all rows?
- Is new column nullable when required for rollout?
- Is constraint added safely?
- Is index creation operationally safe?
- Can migration retry?

Backfills
- Is job idempotent?
- Is progress tracked?
- Is work batched?
- Is load bounded?
- Can it restart?
- Can partial completion be detected?
- Are bad rows handled?
- Is rollback or repair possible?

Data integrity
- Are database constraints present where needed?
- Are uniqueness assumptions enforced?
- Are foreign keys appropriate?
- Are transactions correct?
- Can partial write occur?
- Can concurrent requests violate invariant?
- Are deletes cascading safely?

Transactions
- Is transaction scope correct?
- Are external network calls inside transaction unnecessarily?
- Is rollback tested?
- Is commit ordering correct?
- Is transaction too large?
- Are locks held longer than necessary?
- Can retry duplicate side effects?

Concurrency
- Are shared values synchronized?
- Is read-modify-write atomic?
- Is optimistic locking needed?
- Is pessimistic locking needed?
- Is distributed coordination required?
- Is ordering guaranteed where assumed?
- Are races covered by constraints?
- Can duplicate work occur?

Async
- Are tasks awaited?
- Are detached tasks owned?
- Are failures observed?
- Is cancellation propagated?
- Are resources cleaned?
- Is concurrency bounded?
- Can one request create unbounded tasks?

Retries
- Are only transient failures retried?
- Are attempts bounded?
- Is backoff present?
- Is jitter useful?
- Is operation idempotent?
- Can retry duplicate payment or write?
- Is deadline respected?
- Is cancellation respected?

Timeouts
- Do external calls have timeouts?
- Are database waits bounded where needed?
- Is timeout value reasonable?
- Is timeout distinguishable from underlying failure?
- Is deadline propagated?

Maintainability
- Are names clear?
- Are functions cohesive?
- Are side effects obvious?
- Is coupling reasonable?
- Is code understandable without hidden context?
- Are abstractions justified?
- Is unnecessary complexity introduced?
- Is domain language consistent?

Naming
- Do names describe intent?
- Do boolean names read clearly?
- Do units appear where needed?
- Is ownership clear?
- Are side-effecting methods named clearly?
- Avoid vague names such as data, process, handle when domain term exists.

Functions
- Is function doing one cohesive job?
- Are parameters reasonable?
- Are hidden dependencies avoided?
- Are return values clear?
- Are errors clear?
- Are side effects mixed with calculations unnecessarily?

Classes / modules
- Is responsibility cohesive?
- Are unrelated concerns mixed?
- Is dependency direction sensible?
- Is public surface small?
- Is shared mutable state minimized?
- Is lifecycle clear?

Abstractions
- Does abstraction remove real duplication or isolate real variation?
- Is abstraction premature?
- Are interfaces meaningful?
- Is generic framework needed?
- Does indirection make debugging harder?
- Could simpler code work?

Duplication
- Is duplicated logic genuinely same behavior?
- Would copies change together?
- Is extraction clearer?
- Avoid deduplication that combines unrelated policies.
- Avoid copy-paste bugs.

Comments
- Do comments explain why?
- Are comments still accurate?
- Is commented-out code removed?
- Is surprising business rule documented?
- Could important rule be represented by test?

Performance
- Does change affect hot path?
- Is input size known?
- Are repeated scans introduced?
- Are nested loops acceptable?
- Are allocations excessive?
- Is caching justified?
- Is serialization cost relevant?
- Is algorithmic complexity reasonable?

Database performance
- Is N+1 introduced?
- Are indexes available?
- Are queries selective?
- Is query plan likely to change?
- Is large table scanned?
- Is pagination required?
- Are large results materialized unnecessarily?

Network performance
- Is fan-out bounded?
- Are calls parallelized safely where useful?
- Is batching possible?
- Are payloads bounded?
- Is compression appropriate?
- Are retries amplifying traffic?
- Are timeouts configured?

Memory
- Are large objects retained?
- Are collections bounded?
- Are streams preferable?
- Are caches bounded?
- Are temporary buffers excessive?
- Can request trigger memory spike?

API compatibility
- Are fields removed?
- Are field types changed?
- Are required fields added?
- Are enum values narrowed?
- Is default behavior changed?
- Are error codes changed?
- Are status codes changed?
- Are units changed?

Event compatibility
- Are old consumers supported?
- Are new fields optional?
- Are field meanings stable?
- Is schema versioning needed?
- Can historical events still replay?
- Are ordering assumptions preserved?

CLI compatibility
- Are options renamed?
- Are exit codes changed?
- Is machine output changed?
- Is stderr / stdout behavior changed?
- Are defaults changed?
- Are scripts likely to depend on behavior?

Configuration
- Are names stable?
- Are defaults safe?
- Are old keys supported if required?
- Are secrets handled safely?
- Is precedence unchanged?
- Are invalid values rejected?

Observability
- Will failure be visible?
- Are useful logs present?
- Are logs structured where appropriate?
- Are request / correlation IDs preserved?
- Are new critical operations measured?
- Are sensitive values excluded?
- Is important failure counted?

Logging
- Avoid duplicate log-and-rethrow.
- Avoid secrets.
- Avoid personal data where unnecessary.
- Include safe context.
- Use appropriate severity.
- Avoid noisy logs in hot loops.
- Avoid logging successful high-volume operations excessively.

Metrics
- Is success rate measurable?
- Is failure rate measurable?
- Is latency measurable?
- Is retry count measurable?
- Is queue depth measurable?
- Is saturation measurable?
- Is metric cardinality bounded?

Alerts
- Does new critical dependency need detection?
- Is alert actionable?
- Is threshold meaningful?
- Is owner clear?
- Avoid alerting on every recoverable error.

Deployment
- Can old and new versions coexist?
- Is feature flag needed?
- Is staged rollout useful?
- Is migration order correct?
- Are services deployed in compatible order?
- Is cache format compatible?
- Is rollback possible?

Feature flags
- Is default safe?
- Is both behavior tested?
- Is flag scoped?
- Is owner defined?
- Is removal planned?
- Can flag disable broken feature quickly?
- Avoid permanent forgotten flags.

Rollback
- Can previous application version run?
- Is schema backward compatible?
- Has irreversible data already changed?
- Can feature be disabled?
- Are external side effects reversible?
- Is emergency procedure known?

Operations
- Does support team need documentation?
- Does runbook change?
- Does monitoring dashboard need update?
- Are manual recovery steps required?
- Are failure modes understood?

Review comments
- State problem clearly.
- Explain why it matters.
- Suggest direction where useful.
- Distinguish blocker from suggestion.
- Ask questions when context is missing.
- Avoid personal language.
- Avoid vague criticism.
- Prefer actionable comments.

Blocking comments
- Correctness defect.
- Security issue.
- Data-loss risk.
- Broken public contract.
- Missing critical test.
- Unsafe migration.
- Unhandled high-impact failure.
- Serious performance regression.

Suggestions
- Naming improvement.
- Small readability improvement.
- Alternative abstraction.
- Optional cleanup.
- Future refactor.
- Nonessential style preference.

Automated checks
- Formatting automated.
- Lint automated.
- Type checking automated.
- Unit tests automated.
- Integration tests automated where practical.
- Static security checks automated.
- Dependency scanning automated.
- Build verification automated.

Review process
- Author self-reviews diff.
- CI passes before human review where practical.
- Reviewer understands intent.
- Reviewer classifies risk.
- Reviewer checks highest-risk areas first.
- Author resolves blockers.
- New changes are re-reviewed where necessary.
- Approval means risk is acceptable.

High-risk changes
- Authentication.
- Authorization.
- Payments.
- Encryption.
- Secrets.
- Data deletion.
- Database migration.
- Concurrency.
- Public API.
- Infrastructure dependency.
- Security-sensitive parser.
- File permissions.
- Privileged operations.

High-risk review
- Require experienced reviewer.
- Consider second reviewer.
- Consider security specialist.
- Require explicit tests.
- Require failure-path tests.
- Review rollback.
- Review observability.
- Consider staged rollout.
- Monitor after deployment.

Low-risk review
- Keep process lightweight.
- Do not require security ceremony for typo-only documentation.
- Rely on automation for mechanical checks.
- Preserve fast feedback.

Author self-review
- Read complete diff before requesting review.
- Remove debug code.
- Remove unrelated changes.
- Check test names.
- Check comments.
- Check TODOs.
- Check secrets.
- Check generated files.
- Explain risky areas in description.

Before approval
- Intent understood.
- Correctness checked.
- Tests meaningful.
- Security reviewed at appropriate depth.
- Data impact understood.
- Compatibility understood.
- Maintainability acceptable.
- Performance risk acceptable.
- Deployment safe.
- Rollback understood.

Final review
- Does this change solve the stated problem?
- Is the diff focused?
- Is important behavior correct?
- Are edge cases covered?
- Are failure paths safe?
- Are tests meaningful?
- Is authorization correct?
- Is input validated?
- Are secrets protected?
- Are injection risks controlled?
- Are migrations safe?
- Is data integrity preserved?
- Is concurrency safe?
- Are public contracts compatible?
- Is code maintainable?
- Is performance reasonable for expected scale?
- Will production failures be visible?
- Are timeouts and retries sensible?
- Can the change be rolled back?
- Is review depth appropriate to blast radius?
- Would another developer understand why this code is safe to merge?

16. FAQ

What should a reviewer check first in a pull request?

Start with intent. Understand what problem is being solved, what behavior should change, and what should remain unchanged. Then verify that the diff actually implements that behavior.

Should every code review use the entire checklist?

No. The checklist is a menu of risk areas. A documentation change does not need the same depth as an authorization, payment, database migration, or concurrency change. Apply deeper review where blast radius is larger.

What security issues are most important during code review?

Pay particular attention to authorization, authentication boundaries, input validation, injection, secrets, sensitive logging, file and command execution, unsafe deserialization, dependencies, and access to sensitive data.

Should reviewers request refactors unrelated to the change?

Usually not as blockers unless existing structure makes the current change unsafe. Unrelated improvements can be documented as suggestions or follow-up work rather than expanding the pull request indefinitely.

How many reviewers should a small team require?

There is no universal number. One knowledgeable reviewer may be adequate for ordinary changes, while authentication, payments, migrations, cryptography, infrastructure, or other high-impact changes may justify a second reviewer or specialist.

Should code review comments include suggested code?

They can when a concrete example makes the concern clearer, but the most important part is explaining the risk or maintainability issue. A reviewer does not need to rewrite the complete solution for every comment.

What does approval mean?

Approval means the reviewer believes the remaining risk is acceptable and the change meets the team's quality bar. It does not mean the reviewer would personally implement every detail in exactly the same way.

Key terms (quick glossary)

Code review

The process of evaluating a proposed code change for correctness, security, maintainability, operational safety, and alignment with the intended behavior.

Pull request

A proposed set of source-control changes submitted for discussion, automated checks, review, and eventual merge.

Blast radius

The amount of the system, user population, data, or infrastructure that could be affected if a change fails.

Blocking comment

A review finding that should be resolved before merge because it affects correctness, security, data safety, compatibility, or another required quality property.

Suggestion

A non-blocking review recommendation that may improve clarity, maintainability, or design without being necessary for safe merge.

Regression test

A test that protects behavior associated with a previously discovered defect so the same defect does not return unnoticed.

Backward compatibility

The ability of newer software or schemas to continue working with existing consumers, data, or older deployed components.

N+1 query

A database access pattern where one initial query is followed by an additional query for each returned item, often causing excessive database traffic.

Idempotency

The property that repeating an operation does not create unintended additional effects beyond the intended result.

Feature flag

A runtime control used to enable or disable behavior independently from deploying the underlying code.

Static analysis

Automated examination of source or compiled code without executing the full application, commonly used to identify bugs, style problems, and some security weaknesses.

Authorization

The decision about whether an authenticated identity may perform a specific action on a particular resource.

Data migration

A controlled change to stored data or database structure required when application data models evolve.

Rollback

Returning a system to a previous known-good software or configuration state after a problematic deployment.

Observability

The ability to understand a running system through signals such as logs, metrics, traces, and externally visible behavior.

Risk-based review

A review strategy that applies more scrutiny to changes with greater probability or impact of failure.