Tldr
Code review is a structured examination of a proposed software change by someone other than its author. A pull request provides a place to propose, discuss, check, and merge a branch on platforms such as GitHub. Automated checks and human review catch different kinds of problems, so a sound workflow uses both and adjusts its controls to the risk of the change.
What code review is
Code review is the examination of source code and related material before or after it becomes part of a shared codebase. Most teams review proposed changes before merging them. The reviewer looks for defects, unclear logic, security risks, missing tests, maintainability problems, and inconsistencies with the intended design.
The material under review can include:
- production code
- tests
- database migrations
- infrastructure and network configuration
- documentation
- dependency changes
- build and deployment files
A reviewer needs to understand the purpose of the change as well as its lines of code. Correct syntax cannot compensate for a misunderstood requirement.
Code review is a recurring development and maintenance practice. It is not a separate SDLC phase that happens once near release. Teams may review a design before implementation, each code change during development, urgent fixes during operations, and retirement scripts during decommissioning.
Why teams review changes
Review adds an independent perspective. Authors carry the context of how they built the solution and can unconsciously fill in missing details. A reviewer encounters the change more like a future maintainer.
Review can help a team:
- detect incorrect assumptions and overlooked cases
- keep interfaces, naming, and structure understandable
- identify security, privacy, reliability, and operational concerns
- check that tests address the important behaviours
- spread knowledge of the system
- record why a significant decision was made
- maintain shared engineering standards
Review reduces risk; it does not eliminate it. Two people can miss the same defect, and approval does not show that the assembled system works in production conditions.
Forms of code review
| Form | How it works | Useful context |
|---|---|---|
| Pull-request review | A change is proposed on a collaboration platform and discussed asynchronously | Most routine changes in a shared repository |
| Pair programming | Two developers work on the same change together and review decisions continuously | Complex logic, learning, or rapid collaboration |
| Synchronous walkthrough | The author explains a change while reviewers ask questions | High-risk or unfamiliar changes that need discussion |
| Formal inspection | Defined roles, preparation, checklists, and recorded findings are used | Regulated or safety-critical work |
| Post-commit review | A change is examined after it enters the shared codebase | Low-risk workflows or urgent fixes, usually with additional controls |
The appropriate form depends on impact, urgency, regulation, team size, and the cost of failure. A minor documentation correction and a change to authentication logic do not need identical review depth.
Who participates?
The author creates the change and supplies the context and evidence needed to assess it. A reviewer evaluates the change and gives actionable feedback. A maintainer or code owner has recognized responsibility for a repository or a particular area of it.
Review is not restricted to a senior developer checking a junior developer’s work. Peers can review one another. A specialist may be needed when a change affects cryptography, accessibility, data privacy, networking, database performance, or production operations. Some organizations require approval from designated code owners for sensitive files.
An approval means that a reviewer considers the proposal acceptable within the scope they examined. It does not transfer responsibility away from the author or make the reviewer solely accountable for every future outcome.
Git and collaboration platforms
Pull-request discussions use several related terms:
- Version control records changes to files and preserves a history that people can compare and share.
- Git is a distributed version-control system. Each developer can have a local repository containing files and history.
- A repository is the stored project and its version history.
- A branch is a movable name for a line of development. It lets work progress separately from another line, such as
main. - A commit records a snapshot of selected changes in the local repository, together with metadata such as its author and message.
- A remote is a named connection to another repository, often hosted on a server.
- Push transfers local commits and references to a remote repository.
- GitHub and GitLab are collaboration platforms that host Git repositories and add features such as issue tracking, automated checks, and review.
Git does not itself define a pull request. GitHub calls the proposal a pull request, while GitLab commonly calls it a merge request. Both provide a review conversation around changes that Git records.
The pull-request workflow
flowchart LR A["Create a branch"] --> B["Edit and test locally"] B --> C["Commit a coherent snapshot"] C --> D["Push commits to a remote"] D --> E["Open or update a pull request"] E --> F["Automated checks and human review"] F --> G["Revise the change if needed"] G --> C F --> H["Merge the approved change"] H --> I["Delivery pipeline or later release"]
The exact commands and platform screens differ, but the underlying sequence is stable:
- Create a branch for a focused change.
- Edit the files and check the result locally.
- Stage the intended files. Staging selects which current changes the next Git commit will record.
- Commit a coherent snapshot with a useful message.
- Push the commits to a remote repository.
- Open a pull request proposing that one branch be merged into another.
- Run automated checks and request the appropriate reviewers.
- Respond to feedback with discussion or additional commits.
- Merge when the agreed review and repository rules are satisfied.
- Release through the project’s normal delivery process.
On GitHub, the branch receiving the change is the base branch and the branch supplying the change is the head branch. The base is often main, but repositories can use different names and workflows.
A pull request does not push code. Git push transfers the commits. The pull request then proposes merging the head branch into the base branch and gives collaborators a place to review that proposal.
Merging into a default branch also does not necessarily deploy the software. Deployment depends on the project’s delivery pipeline, environment, approvals, and release strategy.
What automation checks
A check is an automated result attached to a change. Checks may be run by a continuous integration system, usually shortened to CI. CI is the practice of integrating small changes frequently and verifying the combined code through an automated build and tests.
Common checks include:
- compiling or building the software
- unit and integration tests
- formatting and linting
- dependency and vulnerability scanning
- static analysis
- infrastructure validation
- packaging
A linter examines source code for selected errors and style rules without running the complete program. Static analysis inspects code or related artifacts for patterns that may indicate defects, security weaknesses, or quality problems.
Automation is consistent and quick for rules that a tool can express. Human review supplies context and judgment:
| Automated checks are suited to | Human review is suited to |
|---|---|
| Repeating deterministic rules on every change | Assessing whether the change solves the intended problem |
| Running large suites of prepared tests | Finding missing cases that nobody encoded as tests |
| Enforcing formatting and detectable policy | Judging clarity, maintainability, and design trade-offs |
| Detecting known vulnerable patterns or dependencies | Examining business, privacy, security, and operational context |
A green build means the configured checks passed. It does not show that the checks were sufficient.
Preparing a change for review
The author should review the change before asking someone else to do so. A strong proposal usually:
- addresses one coherent purpose
- explains the problem and the chosen approach
- separates relevant changes from unrelated cleanup
- includes new or updated tests where behaviour changes
- describes how the result was verified
- identifies risks, migrations, rollout needs, and rollback options
- links the requirement or issue that provides context
- highlights areas where the author wants particular attention
A diff is the line-by-line representation of what changed between two versions. Generated files, formatting noise, and unrelated renaming can make the meaningful part of a diff difficult to see.
Prefer focused changes
Small, self-contained changes are usually easier to understand, test, review thoroughly, merge, and reverse. “Small” describes conceptual scope as much as line count. A one-line authorization change may carry more risk than a large generated file.
Large work can often be divided into independent preparation, behaviour, and cleanup changes. Each merged change should leave the system in a valid state. Splitting a change carelessly can create incomplete interfaces or temporarily broken behaviour.
Reviewing a change
A reviewer can begin with the purpose before reading individual lines:
- What requirement or problem does this change address?
- Is the proposed behaviour appropriate?
- Does the overall design fit the surrounding system?
- What can fail, and how will that failure be handled?
- Is the change understandable to someone maintaining it later?
- Do the tests and checks provide suitable evidence?
Then examine relevant details:
- Correctness: Are calculations, conditions, state changes, and error paths sound?
- Security and privacy: Are permissions, validation, secrets, personal data, and audit needs handled?
- Reliability: Are timeouts, retries, partial failures, concurrency, and recovery considered where relevant?
- Maintainability: Are responsibilities clear and duplication controlled?
- Compatibility: Could the change break existing callers, stored data, configuration, or supported environments?
- Observability: Will logs, metrics, traces, or alerts make important production behaviour visible?
- Performance: Could the change cause unacceptable response time, memory use, network traffic, or database load?
- Tests: Do they cover the intended behaviour, boundaries, failures, and regression risk?
- Documentation: Do users, operators, or future developers need an explanation or migration instruction?
The checklist should be adjusted to the change. Asking about database transactions in a spelling correction adds noise. Skipping authorization analysis in an access-control change creates risk.
Giving useful feedback
Review comments are easier to act on when they explain the concern and its consequence. Compare:
This is wrong.
with:
If two requests update this value at the same time, the later write can erase the earlier one. Could we use the existing transaction helper here?
The second comment identifies the failure condition, explains the impact, and suggests a direction without pretending there is only one solution.
It also helps to distinguish:
- Blocking concern: A defect or risk that must be resolved before approval.
- Suggestion: An improvement worth considering that does not prevent approval.
- Question: A request for missing context or clarification.
- Nit: A minor, optional point, usually about polish.
Technical facts, requirements, and agreed standards should carry more weight than personal taste. When several solutions are reasonable, discuss their trade-offs. If a long comment thread stops producing understanding, a short conversation may help; record the resulting decision in the pull request for future readers.
Authors should treat review as an examination of the change rather than of their ability. Reviewers should address the code and its effects, avoid dismissive language, and recognize sound decisions. Respect improves candour, which improves the chance that real risks will be raised.
Review outcomes on GitHub
GitHub provides three general review outcomes:
- Comment: Submit feedback without explicitly approving or requesting changes.
- Approve: Indicate that the proposed changes are acceptable.
- Request changes: Submit feedback that should be addressed before merging.
Repository rules determine what is required. A “request changes” review blocks merging only when the branch protection or ruleset is configured to require approving reviews and the relevant conditions apply. Teams should define required reviewers, checks, and exceptions deliberately rather than assume the platform’s defaults match their risk.
Merge strategies
Merging combines the reviewed branch into the base branch. GitHub can support several strategies, subject to repository settings:
| Strategy | Resulting history | Trade-off |
|---|---|---|
| Merge commit | Preserves the branch’s commits and adds an explicit merge commit | Retains branch context, but history can become visually busy |
| Squash and merge | Combines the pull request into one commit on the base branch | Creates a concise history, but discards the separate commit structure from the branch |
| Rebase and merge | Reapplies individual commits onto the tip of the base branch without a merge commit | Produces a linear history and retains individual changes, but creates new commit identifiers |
A commit identifier, often called a commit hash or SHA, uniquely identifies the recorded Git object. Rebasing creates new commits because their parent history changes.
No merge strategy is universally best. The team should choose a convention that supports traceability, release practices, rollback, and the quality of its commit history.
A practical example
Suppose a team maintains a function that reads an interface status from a configuration line:
def parse_interface_status(line):
name, status = line.split()
return {"interface": name, "status": status}The proposed change works for GigabitEthernet0/1 up. A useful review might ask:
- What should happen when the line is empty or contains extra fields?
- Which status values are valid?
- Should leading and trailing whitespace be accepted?
- Does invalid input produce an error that the caller can handle?
- Are interface names case-sensitive?
- Do tests cover a valid line and representative invalid lines?
These questions connect code to its contract. A contract describes the inputs a component accepts, the result it promises, and how it reports failure. The reviewer need not rewrite the function personally. The author can clarify the requirement, add focused tests, and choose an implementation consistent with the surrounding parser.
Code review, CI, and delivery
Code review and CI are connected but distinct:
- Code review is the human examination and discussion of a proposed change.
- CI integrates changes frequently and applies automated verification.
- Continuous delivery keeps validated software in a state that can be released through a controlled decision.
- Continuous deployment automatically releases every change that satisfies the delivery process.
A team can use pull requests without mature CI. It can also run CI on direct commits without pull requests. Many teams combine them so a change needs both required checks and human approval before merge.
Passing both gates still does not guarantee production readiness. Some changes need performance testing, security assessment, data migration rehearsal, staged rollout, business approval, or operational preparation.
Review and technical debt
Technical debt is future cost or difficulty created by a technical trade-off or shortcut made earlier. A reviewer may identify debt when a change adds duplication, weakens an interface, or postpones necessary error handling.
Practical teams sometimes accept debt to meet an urgent constraint. The decision should be conscious: explain the reason, contain the risk, record follow-up work where appropriate, and avoid calling every maintenance task “debt.” Routine dependency upgrades and ordinary support work can be maintenance without resulting from an earlier shortcut.
Common mistakes
- Reviewing only formatting: Automated tools can handle many style rules, leaving human attention for behaviour and risk.
- Assuming tests prove correctness: Tests cover selected conditions and can contain errors themselves.
- Requesting personal preferences as requirements: Feedback should connect to evidence, standards, or a concrete engineering consequence.
- Approving code that is not understood: Schedule pressure does not turn uncertainty into evidence.
- Submitting unrelated changes together: The meaningful behaviour becomes harder to trace and reverse.
- Using review to redesign the entire feature late: Early design discussion can prevent large rework.
- Treating approval as permission to deploy anywhere: Merge and release controls serve different purposes.
- Restricting review to senior staff: Appropriate knowledge and responsibility matter more than hierarchy alone.
- Letting comments remain unresolved without a decision: Clarify, change the code, document agreement, or use the team’s escalation path.
- Requiring perfection: Review should protect and improve the codebase while allowing useful work to progress.
Where code review fits in the SDLC
- Planning and requirements: Teams identify changes that need specialist or formal approval.
- Design: Early review exposes architectural and security concerns before implementation cost grows.
- Development: Peers examine focused code changes and their tests.
- Verification: Review checks whether the chosen tests and automated evidence address the important risks.
- Deployment: Infrastructure, migration, release, and rollback changes can be reviewed.
- Operations: Incident fixes and reliability improvements receive proportionate scrutiny.
- Maintenance and retirement: Dependency updates, data migrations, archival logic, and access removal remain reviewable changes.
In Agile development, frequent review supports short feedback cycles. Within Scrum, a team may include review and required checks in its Definition of Done. Scrum does not prescribe Git, pull requests, GitHub, or a particular approval policy.
Summary
- Code review is an independent examination of a proposed change and its evidence.
- Review is a continuing practice across the SDLC rather than a single phase.
- Git records commits and transfers them between repositories; platforms such as GitHub add pull-request collaboration.
- A pull request proposes merging a head branch into a base branch.
- Automated checks and human review address different kinds of risk.
- Focused changes are generally easier to understand, test, merge, and reverse.
- Review comments should explain the concern, consequence, and importance.
- Repository rules determine which approvals and checks block merging.
- Merge, deployment, and release are separate decisions.
- Approval reduces risk but does not guarantee that the entire system is correct or production-ready.
Related topics
- Software Development Life Cycle
- SDLC Development Practices
- Unit Testing and Test-Driven Development
- Agile Software Development
- The Scrum Framework