Pull/Merge Requests and Code Review

Introduction and Context

In a collaborative DevOps environment, code is rarely integrated directly into the main branch by its author. Instead, a developer proposes a change through a pull request (PR) on GitHub or a merge request (MR) on GitLab. Although the names differ, both mechanisms support the same broad workflow: a contributor presents changes, reviewers examine them, automated checks provide evidence, authorized people approve or reject the proposal, and the change is eventually integrated into a protected branch.

Pull/merge requests are more than technical mechanisms for combining branches. They are governance and communication instruments. They provide a visible record of what changed, why it changed, who examined it, which tests were run, and when the change became part of the product. A well-managed request connects a requirement or incident to implementation, review, testing, deployment, and, where necessary, rollback.

Code review is the human reasoning component of this process. Automated tools can detect formatting violations, dependency vulnerabilities, failed tests, and some classes of defects. They cannot reliably determine whether a feature solves the correct business problem, whether an interface is understandable, or whether a design is appropriate for its operational context. Effective review therefore combines technical inspection, automated evidence, domain ownership, and respectful professional communication.

This chapter explains how to prepare and review pull/merge requests, use draft requests and approval rules, manage ownership with CODEOWNERS, and maintain traceability from requirements to deployed changes.

Learning Objectives

By the end of this chapter, you should be able to:

1. Explain the purpose and lifecycle of pull requests and merge requests.
2. Prepare a small, reviewable, and traceable change.
3. Distinguish comments, change requests, approvals, and discussions.
4. Conduct constructive reviews using risk-based reasoning.
5. Apply draft reviews, branch protection, approval rules, and status checks.
6. Configure and interpret CODEOWNERS files.
7. Link requirements, issues, commits, requests, builds, deployments, and operational outcomes.
8. Recognize limitations, governance risks, and ethical responsibilities in review systems.

1. The Pull/Merge Request as a Control Point

A pull/merge request is a proposed integration between a source branch and a target branch. The source branch contains the proposed changes; the target branch is the branch into which those changes may be integrated, commonly main, master, or a release branch.

A request normally contains:

• a title and description;
• the source and target branches;
• a comparison showing changed files and lines;
• commits associated with the change;
• linked issues, requirements, or incidents;
• automated build, test, security, and quality results;
• reviewers, assignees, and ownership rules;
• comments, discussions, approvals, and decisions;
• a record of merging, closing, or updating the request.

The request creates a controlled transition from individual development to shared product state. It allows an organization to answer questions such as:

• What problem was being solved?
• What assumptions were made?
• Which files and services were affected?
• What evidence supports the change?
• Who reviewed the technical and operational risks?
• Which policy allowed the change to be merged?

Pull request and merge request terminology

GitHub generally uses pull request, while GitLab generally uses merge request. The difference is mainly product vocabulary. Features vary by platform and configuration, so an exact control—such as required approvals, merge trains, or approval resets—should be checked against the organization’s platform documentation and version.

A request should not be understood as a guarantee of quality. It is a control point whose effectiveness depends on the quality of the change, the competence and independence of reviewers, the reliability of automation, and the organization’s governance rules.

2. Preparing a High-Quality Request

2.1 Start with a small, coherent change

Reviewability is strongly affected by change size and conceptual complexity. A request that changes one well-defined behavior is easier to understand than one that combines a database migration, a user-interface redesign, dependency upgrades, and unrelated refactoring.

A good request generally has:

• one primary purpose;
• a clear relationship to a requirement, issue, or incident;
• a limited and explainable scope;
• tests appropriate to the risk;
• documentation or configuration updates when required;
• no unrelated formatting or cleanup changes.

Small changes are not always possible. A large migration may be necessary, but it should then be divided into safe phases where possible—for example, introduce a backward-compatible schema, deploy code that supports both schemas, migrate data, and remove obsolete behavior later.

2.2 Write an informative title and description

A title should communicate the change rather than merely repeat a ticket number. For example:

Add idempotency protection to payment webhook processing

A useful description typically includes:

A description should state uncertainty honestly. “Unit tests pass” is evidence about unit tests; it is not evidence that the feature is correct in production or that performance is acceptable under load.

2.3 Prepare evidence before requesting review

The author is responsible for making the request reviewable. Before assigning reviewers, the author should:

1. Rebase or update the branch according to team policy.
2. Run relevant local tests and linters.
3. Inspect the complete diff, not only the files recently edited.
4. Remove secrets, credentials, debug output, and temporary files.
5. Confirm that generated files are handled according to repository policy.
6. Explain migrations, feature flags, compatibility, and rollback.
7. Check that continuous integration results are complete and relevant.
8. Identify reviewers with the required technical or domain knowledge.

The author should not use automation as a substitute for reading the diff. A passing pipeline may show that known checks passed; it does not show that the intended requirement was implemented correctly.

3. Draft Requests and the Lifecycle of Review

A draft pull/merge request communicates that work is visible but not yet ready for final approval. Draft status is useful when an author wants early architectural feedback, wants to share progress, or needs a formal place to discuss an incomplete design without implying merge readiness.

A typical lifecycle is:

1. Plan: identify the requirement, risk, and acceptance criteria.
2. Develop: implement the change on a dedicated branch.
3. Open as draft: expose direction and invite early feedback if useful.
4. Refine: add tests, documentation, and operational detail.
5. Mark ready: indicate that the author believes the request meets the team’s review standard.
6. Review: reviewers inspect the change and evidence.
7. Revise: the author responds to findings and updates the branch.
8. Approve and verify: required approvals and automated checks are satisfied.
9. Merge: the platform integrates the change under repository policy.
10. Deploy and observe: delivery and operational monitoring provide further evidence.

Draft status should not be treated as a security boundary. Unless repository permissions prevent it, code in a draft request may still be visible to collaborators. Sensitive information must never be placed in a request merely because it is marked draft.

A request can be marked ready too early. Before doing so, the author should verify that the acceptance criteria are addressed, known failures are explained, the diff is focused, and reviewers have enough context to make a decision.

4. Reviewing Code Constructively

Code review is a technical evaluation and a learning conversation. The reviewer’s responsibility is not to demonstrate superiority or enforce personal preferences. It is to identify material risks, improve maintainability, and confirm that the change satisfies its purpose.

4.1 A risk-based review order

A practical review sequence is:

1. Read the title, description, linked requirement, and acceptance criteria.
2. Understand the architecture and affected components.
3. Examine the overall diff before inspecting individual lines.
4. Check correctness and failure behavior.
5. Check security, privacy, authorization, and input handling.
6. Check tests and whether they cover important risks.
7. Check performance, reliability, observability, and operability.
8. Check maintainability, readability, and consistency.
9. Check documentation, migrations, configuration, and deployment impact.
10. Confirm that the final discussion accurately records decisions.

This order prioritizes consequences over stylistic preferences. A reviewer should first ask whether the change can cause data loss, unauthorized access, service disruption, or incorrect business results. Naming and formatting matter, but they should not obscure higher-risk findings.

4.2 Review comments and their classification

Comments are more useful when they distinguish severity and required action. Teams may use labels such as:

• Blocker: must be resolved before merge because it creates a serious correctness, security, compliance, or operational risk.
• Major: important defect or missing requirement that normally requires correction.
• Minor: worthwhile improvement that does not usually justify blocking integration.
• Question: requests clarification without asserting that the implementation is wrong.
• Suggestion: an optional alternative.
• Praise: identifies a good decision and reinforces effective practice.

A constructive comment is specific, evidence-based, and actionable. Compare:

• Weak: “This is bad.”
• Strong: “If the webhook is retried after the database commit but before the response, this handler creates a second payment record. Can we enforce an idempotency key at the database boundary and add a retry test?”

Reviewers should separate observations from assumptions. “This endpoint accepts an unauthenticated request” is an observation if verified. “Users will never call this endpoint directly” is an assumption that should be supported by architecture or policy.

4.3 Approval, request changes, and discussion

Platform actions differ, but their meanings are commonly similar:

• Approve: the reviewer has examined the change and has no blocking concern within the reviewer’s responsibility.
• Request changes: a blocking issue must be addressed before integration.
• Comment: feedback is provided without changing the formal approval state.
• Resolve discussion: the conversation has reached a documented conclusion, either through a code change or an accepted rationale.

An approval is not universal certification. A security reviewer may approve security aspects while not certifying business correctness. Review responsibility should therefore be explicit, especially for changes involving regulated data, infrastructure, or production access.

5. Technical Review Dimensions

Correctness and requirements

Reviewers should trace implementation behavior to acceptance criteria. Important questions include:

• Does the change implement the stated behavior for normal and exceptional inputs?
• Are boundary conditions handled?
• Are error messages and status codes appropriate?
• Does the code preserve existing compatibility commitments?
• Are transactions, retries, and concurrency handled safely?

Testing

Tests should be proportional to risk. A change to a pure formatting function may need a small unit test; a payment or access-control change may require unit, integration, security, and deployment validation.

Reviewers should inspect what tests prove and what they do not prove. A test that only checks that a function returns a value may not establish correctness if it uses the same flawed assumption as the implementation. Tests should include meaningful failure cases where those cases are plausible.

Security and privacy

Reviewers should consider authentication, authorization, input validation, output encoding, dependency risk, secret handling, logging, and data retention. Logs may themselves expose personal or confidential data. A request that adds telemetry should explain whether the data is necessary, who can access it, and how long it is retained.

Reliability and operations

A production-oriented review considers timeouts, retries, idempotency, resource limits, graceful degradation, health checks, alerting, and rollback. A retry without an idempotency strategy can duplicate an operation. A new feature may require a dashboard or alert before deployment. Configuration defaults should be safe and documented.

Maintainability and design

Reviewers should assess coupling, duplication, abstractions, naming, error handling, and the likely cost of future change. Personal style preferences should not become blocking rules unless they are documented standards with a meaningful maintenance rationale.

6. Approval Rules, Branch Protection, and Ownership

A repository may define policies that must be satisfied before integration. Common controls include:

• required status checks;
• a minimum number of approvals;
• approval from designated teams or code owners;
• prohibition of direct pushes to protected branches;
• dismissal or reset of approvals after new commits;
• resolution of all blocking discussions;
• successful deployment to a test environment;
• signed commits or verified identities;
• restrictions on who may merge.

These controls are safeguards, not substitutes for judgment. Excessively rigid rules can encourage rubber-stamping, while weak rules can allow unreviewed production changes. Policies should reflect risk and be periodically evaluated.

Approval independence

For sensitive changes, the author should not be the sole approver. Separation of duties reduces the possibility that an error or conflict of interest will pass unnoticed. However, independence must be balanced with team capacity. A rule that requires unavailable specialists for every trivial change can create unsafe delays or encourage workarounds.

Approval invalidation after updates

A critical governance question is whether an approval remains valid after the source branch changes. If a new commit modifies code covered by the review, the platform may dismiss approvals or require re-approval. This protects against merging materially different code from the code that was approved. Teams should understand whether their platform resets approvals automatically and should avoid treating an old approval as blanket authorization for later changes.

7. CODEOWNERS and Ownership Management

A CODEOWNERS file maps paths to users or teams responsible for reviewing changes. It can be stored in a repository location supported by the platform, such as a designated configuration directory. A simplified example is:

# Documentation and platform ownership
/docs/              @documentation-team
/infrastructure/    @platform-team
/services/payments/ @payments-team
*.tf                @platform-team

The exact syntax and precedence rules vary by platform. In many implementations, the last matching pattern determines the applicable owner, and ownership rules are evaluated against changed paths. Teams must verify platform-specific behavior rather than assuming that all Git hosting services interpret patterns identically.

CODEOWNERS is valuable because it makes expected responsibility visible and can require approval from an owning team. It does not prove that the named team actually reviewed the code deeply. Ownership should be maintained as an organizational asset:

• use teams rather than frequently changing individual names where appropriate;
• keep ownership aligned with real system responsibility;
• define backup owners and escalation paths;
• review rules when repositories or services move;
• test patterns with representative file paths;
• avoid assigning so many owners that responsibility becomes ambiguous;
• protect the ownership file itself with suitable review requirements.

Ownership is not the same as authorship. A service team may own operational reliability while a security team owns security review. Cross-functional review is especially important for changes that cross system boundaries.

8. Traceability from Requirement to Change

Traceability is the ability to follow a change through its lifecycle. A practical chain is:

Requirement or incident → issue → branch → commits → pull/merge request → reviews and checks → merge commit → build artifact → deployment → operational evidence

Not every organization uses exactly these objects, but the principle is consistent: each transition should leave enough evidence to reconstruct what happened.

Traceability techniques

1. Link the request to a work item. Include the issue, requirement, incident, or change record.
2. Use descriptive commits. Commits should explain meaningful units of work; avoid relying exclusively on opaque ticket numbers.
3. Record acceptance criteria. The reviewer should be able to compare intended behavior with implementation evidence.
4. Preserve automation evidence. Store links or records for builds, tests, scans, and deployments according to retention policy.
5. Link artifacts to source revisions. A deployed artifact should identify the commit or immutable revision from which it was built.
6. Record exceptions. If a policy was bypassed, document who authorized it, why it was necessary, and what compensating control was used.
7. Connect operational results. Incidents, monitoring observations, and rollback events should point back to the deployed change.

Traceability is important for debugging, audits, regulated environments, incident response, and learning. It should not become bureaucracy for its own sake. The objective is reliable evidence, not a large quantity of disconnected links.

9. A Worked Review Example

Consider an illustrative change to a payment service. The requirement states: “The service must process a provider webhook at least once without creating duplicate payment records when the provider retries a notification.”

The author opens a request titled “Prevent duplicate payment records during webhook retries.” The description links the requirement, explains the database constraint, lists unit and integration tests, and provides a rollback plan.

A reviewer examines the implementation and identifies three elements:

1. The handler extracts a provider event identifier.
2. It inserts a payment record with that identifier.
3. A uniqueness constraint rejects repeated identifiers.

The reviewer asks what happens when the second request receives a uniqueness violation. If the handler returns a server error, the provider may continue retrying even though the first request succeeded. The reviewer therefore comments:

“The uniqueness constraint prevents duplicate storage, but a repeated event currently returns an error. Can the handler treat an already-processed event as a successful, idempotent response? Please add a test covering two deliveries of the same event and document the response behavior.”

The author updates the handler and adds a test. A second reviewer from the payments team checks the transaction boundary and confirms that the event identifier is stored atomically with the payment state. The pipeline runs unit, integration, dependency, and static-analysis checks. The request receives required approvals, merges, and deploys behind a feature flag.

After deployment, monitoring shows no duplicate records and normal webhook retry rates. The request links the resulting artifact and deployment. If an incident later occurs, engineers can follow the evidence from the incident to the deployment, request, requirement, review discussion, and exact source revision.

Learning points

• A database constraint is valuable but does not by itself define correct API behavior.
• Review should consider retries, failure timing, and external-system behavior.
• Tests should represent realistic delivery patterns, not only the happy path.
• Domain ownership improves review quality for high-consequence changes.
• Traceability connects implementation decisions with operational outcomes.

10. Challenges and Limitations

Review fatigue and rubber-stamping

High request volume, large diffs, and interruptions reduce attention. Teams can reduce fatigue through smaller changes, risk-based reviewer assignment, clear ownership, and automation that removes repetitive checks. Metrics such as approval count should not be used alone to measure review quality; they can encourage superficial behavior.

Large or generated diffs

Generated files, lock files, vendor code, and formatting changes may obscure important logic. Repository conventions should identify generated content, and authors should separate mechanical changes from behavioral changes where practical.

Social and cultural risks

Written comments can be interpreted more harshly than intended. Reviewers should critique the code and evidence, not the character or intelligence of the author. Teams should make room for questions, recognize power differences, and provide escalation routes for harassment, discrimination, or retaliation.

False confidence from automation

A green pipeline indicates that configured checks passed under their tested conditions. It does not establish absence of all defects. Flaky tests, incomplete coverage, incorrect test assumptions, and untested production conditions can produce false confidence. Continuous integration should therefore be treated as evidence with limits.

Bypasses and emergency changes

Production incidents may require an emergency path. Emergency access should be narrowly controlled, logged, reviewed retrospectively, and followed by corrective work. A permanent bypass for convenience undermines the purpose of branch protection.

11. Ethics, Governance, and Contemporary Practice

Review systems process information about employees, work patterns, and decisions. Organizations should collect only metrics needed for legitimate improvement and should avoid using simplistic measures—such as lines changed or review time—to judge individual worth. Automated review tools and AI coding assistants can suggest defects, summarize diffs, generate tests, or identify dependencies, but their output requires human validation.

AI-generated code may reproduce insecure patterns, licensing concerns, biased assumptions, or fabricated APIs. Authors remain responsible for understanding and testing submitted code. Reviewers should ask whether generated content has been inspected for security, privacy, license compatibility, and maintainability. Confidential source code and personal data should not be sent to external tools unless organizational policy and contractual safeguards permit it.

Contemporary teams increasingly combine protected branches, ephemeral test environments, dependency scanning, secret detection, signed artifacts, deployment previews, and policy-as-code. These practices improve evidence and consistency, but they also create configuration risk. A wrongly configured policy can block valid work or permit unsafe work. Governance must therefore include periodic testing, ownership, documented exceptions, and review of the controls themselves.

Best-Practice Checklist

Before opening a request:

• Is the change tied to a clear requirement, incident, or decision?
• Is the scope coherent and as small as practical?
• Are tests and documentation included?
• Have secrets, debug statements, and unrelated edits been removed?
• Are migration, compatibility, deployment, monitoring, and rollback addressed?

During review:

• Read the requirement and overall diff first.
• Prioritize correctness, security, reliability, and operational risk.
• State evidence and expected action clearly.
• Distinguish blocking findings from optional suggestions.
• Verify that approvals correspond to the current revision.
• Resolve discussions with a documented decision.

Before merge:

• Required owners have reviewed the relevant paths.
• Status checks are complete and meaningful.
• Required discussions are resolved.
• The final revision is the one reviewed.
• Traceability links and deployment information are available.

Chapter Summary

Pull requests and merge requests provide a controlled path from individual development to shared product state. Their value comes from combining a focused change, clear context, automated evidence, human judgment, approval rules, and traceability. Draft requests support early collaboration without implying readiness. Constructive review prioritizes correctness, security, reliability, testing, and operational impact over personal style preferences. Branch protection and approval rules establish minimum governance controls, while CODEOWNERS makes technical responsibility visible and can require domain approval. Effective traceability links requirements and incidents to branches, commits, reviews, checks, artifacts, deployments, and operational outcomes. These mechanisms remain limited by human attention, automation quality, configuration accuracy, and organizational culture; they must therefore be maintained and reviewed as part of the DevOps system.

Key Terms

Pull request: A proposed change from a source branch into a target branch, commonly used by GitHub.

Merge request: GitLab’s term for a proposed branch integration.

Source branch: The branch containing proposed changes.

Target branch: The branch into which changes may be integrated.

Draft request: A request visible for collaboration but not yet declared ready for final review.

Code review: Systematic examination of a proposed change by one or more reviewers.

Approval: A reviewer’s formal indication that no blocking concern remains within their responsibility.

Change request: A formal indication that a blocking issue must be addressed before merge.

Branch protection: Repository rules restricting direct or insufficiently reviewed changes.

Status check: An automated result, such as a build, test, scan, or deployment check.

CODEOWNERS: A path-to-owner mapping used to identify or require responsible reviewers.

Traceability: The ability to follow a change through requirements, implementation, validation, and operation.

Acceptance criteria: Conditions that define when a requirement is satisfied.

Idempotency: The property that repeated processing of the same operation has the intended single-effect outcome.

Separation of duties: Distribution of responsibilities so that one person does not control all stages of a sensitive change.

Protected branch: A branch subject to restrictions and required controls.

Rollback: Reversal or mitigation of a deployed change when it causes unacceptable effects.

Policy bypass: An exceptional action that circumvents a normal control and requires authorization and documentation.

Review Questions

Short Answer Questions

1. What is the principal difference between a pull request and a merge request?
2. Why should a request normally contain one coherent purpose?
3. What is the purpose of marking a request as draft?
4. Give two examples of evidence that may be attached to a request.
5. What problem does a CODEOWNERS file address?

Long Answer Questions

1. Explain the complete lifecycle of a pull/merge request from planning through deployment and observation.
2. Describe a risk-based approach to reviewing a change, including correctness, security, testing, and operations.
3. Explain how branch protection and approval rules contribute to governance, and discuss their limitations.
4. Discuss how constructive review comments improve both software quality and team learning.
5. Explain how traceability can support incident response, auditing, and continuous improvement.

Analytical / Application Questions

1. A request changes authentication logic, database schema, deployment configuration, and unrelated formatting in one large diff. Analyze the risks and propose a safer review and integration strategy.
2. A reviewer approved a request, but the author subsequently added a commit changing authorization behavior. Determine what governance questions should be asked before merge and what review action is appropriate.
3. A webhook service prevents duplicate database rows but returns an error when a provider retries an already-processed event. Analyze the operational consequences and propose implementation, testing, and review requirements.

Multiple Choice Questions with Answers

Question 1

What is the primary purpose of a pull/merge request?

A. To provide a controlled proposal for integrating changes into a target branch

B. To replace all automated testing

C. To permanently prevent branch creation

D. To store production secrets

Correct Answer: A. To provide a controlled proposal for integrating changes into a target branch

Question 2

What does draft status generally communicate?

A. The change has been merged

B. The change is visible but not yet ready for final approval

C. The change has passed every production test

D. The change is hidden from repository collaborators

Correct Answer: B. The change is visible but not yet ready for final approval

Question 3

Which review comment is most constructive?

A. This code is terrible

B. I would write it differently

C. The retry path may create duplicate records; can you add idempotency handling and a retry test?

D. Everyone knows this pattern is wrong

Correct Answer: C. The retry path may create duplicate records; can you add idempotency handling and a retry test?

Question 4

What is a likely purpose of branch protection?

A. To require specified controls before changes enter a protected branch

B. To prevent all users from reading repository history

C. To automatically design application architecture

D. To eliminate the need for source control

Correct Answer: A. To require specified controls before changes enter a protected branch

Question 5

What does CODEOWNERS primarily provide?

A. Automatic encryption of every source file

B. A mapping between paths and responsible reviewers or teams

C. A list of all runtime dependencies

D. A replacement for acceptance criteria

Correct Answer: B. A mapping between paths and responsible reviewers or teams

Question 6

Why might an approval be dismissed after a new commit?

A. The repository has no branches

B. The new commit may materially differ from the reviewed revision

C. The author changed their email address

D. Automated tests are never required

Correct Answer: B. The new commit may materially differ from the reviewed revision

Question 7

Which sequence best represents traceability?

A. Deployment → requirement → unrelated file → reviewer

B. Requirement → issue → branch → request → checks → artifact → deployment

C. Comment → password → production database → branch

D. Reviewer → formatting → secret → incident

Correct Answer: B. Requirement → issue → branch → request → checks → artifact → deployment

Question 8

A uniqueness constraint prevents a duplicate webhook record, but the duplicate request returns a server error. What is the main concern?

A. The provider may retry unnecessarily despite the operation already succeeding

B. The database will necessarily lose all records

C. The source branch will be deleted automatically

D. The request cannot contain documentation

Correct Answer: A. The provider may retry unnecessarily despite the operation already succeeding

Question 9

Which statement about automated checks is most accurate?

A. A green pipeline proves that no defect exists

B. Automated checks provide evidence within their configured scope and limitations

C. Automated checks make code review unnecessary in every case

D. Automated checks are valid only for documentation changes

Correct Answer: B. Automated checks provide evidence within their configured scope and limitations

Question 10

Which practice best supports responsible use of AI-generated code?

A. Merge generated code without reading it if the build passes

B. Assume generated code has no licensing or security implications

C. Validate the code for correctness, security, privacy, licensing, and maintainability

D. Send confidential source code to any public tool

Correct Answer: C. Validate the code for correctness, security, privacy, licensing, and maintainability