Code Review Guidelines for Humans

Standard Operating Procedure: Code Review Guidelines for Humans

Document ID: SOP-ENG-402-REV4
Classification: Enterprise Engineering Standards / Mandatory Compliance

---

1. Document Control & Revision History

---

2. Executive Summary & Scope

2.1 Clinical Statement of Operational Boundaries

This Standard Operating Procedure (SOP) defines the mandatory lifecycle, governance frameworks, and behavioral protocols for human-executed code reviews within enterprise software development environments. The objective is to eliminate systemic technical debt, enforce zero-trust security postures, prevent regression anomalies, and maintain strict adherence to regulatory frameworks including SOC2 Type II, ISO/IEC 27001, and HIPAA (where applicable).

2.2 Scope

This SOP applies unconditionally to:

• All software engineers, senior engineers, staff engineers, and tech leads performing or undergoing peer code reviews.
• All production, staging, and infrastructure-as-code (IaC) repositories owned, maintained, or operated by the enterprise.
• Pull Requests (PRs), Merge Requests (MRs), and hotfix patches submitted across all business units.

---

3. Prerequisites & Tool Requirements

Execution of this SOP requires mandatory licensing, tooling access, and credentialing prior to interaction with enterprise codebases.

• Hardware & Environment: Enterprise-managed workstation utilizing FIPS 140-2 compliant disk encryption, restricted USB peripherals, and secure VPN tunnels.
• Software & Access Control:
  • Git-based version control platform (GitHub Enterprise / GitLab Ultimate) configured with hardware-token Multi-Factor Authentication (MFA/U2F).
  • Integrated Development Environment (IDE) with enterprise-approved static analysis linters (e.g., SonarQube, Snyk, Semgrep).
  • Automated CI/CD pipeline execution status reporting tools.
• Certifications & Training:
  • Completion of Enterprise Secure Coding Practices (Module SEC-101).
  • Passing grade on the Human Code Review Calibration Assessment (Certification ID: HCR-CAP-04).

---

4. Roles & Responsibilities (RACI Matrix Table)

The following RACI matrix dictates accountability across the code review lifecycle:

• Responsible: The role that performs the activity.
• Accountable: The sole decision-maker with final veto power.
• Consulted: The subject matter expert providing advisory input.
• Informed: The role kept updated on progress and outcomes.

---

5. Step-by-Step Operational Procedure

Phase 1: Author Preparation & Submission (Pre-Review)

• Ensure all code changes map to an accepted, scoped enterprise tracking ticket (Jira/Linear).
• Execute local unit tests, integration tests, and static analysis linters; confirm 100% pass rate.
• Verify that branch naming conventions strictly follow type/ticket-id-short-description (e.g., feat/SEC-402-oauth-refresh).
• Complete the mandatory Pull Request template, explicitly linking architectural diagrams, threat models, and test coverage metrics.
• Self-review the entire diff line-by-line to strip out debugging statements, uncommitted files, and extraneous whitespace.

Phase 2: Reviewer Triage & Initial Assessment

• Acknowledge review assignment within the defined SLA window (Standard Priority: < 4 business hours; P0/Hotfix: < 30 minutes).
• Verify that automated CI/CD pipeline checks (build status, dependency vulnerability scans, test coverage floors) have passed cleanly. Do not begin line-by-line review if pipeline gates are failing.
• Evaluate the scope of the Pull Request; if the diff exceeds 400 lines of code (excluding auto-generated artifacts), request the author to split the PR into logically isolated units.
• Review the business logic against the ticket acceptance criteria to ensure the implementation solves the correct core problem.

Phase 3: Deep Technical Evaluation & Security Audit

• Architecture & Design: Check for adherence to SOLID principles, modularity, and separation of concerns. Ensure no circular dependencies are introduced.
• Security (OWASP Top 10): Inspect for injection vulnerabilities, insecure direct object references, improper data sanitization, hardcoded secrets, and broken authentication logic.
• Data Integrity & Concurrency: Assess database migration safety (ensuring zero-downtime, backward-compatible migrations), race condition risks, and thread safety.
• Error Handling & Logging: Verify that exceptions are caught gracefully, actionable errors are logged without exposing Personally Identifiable Information (PII) or secrets, and resources are correctly disposed of (defer/finally blocks).
• Test Coverage: Ensure newly introduced code paths possess unit and integration test coverage meeting the enterprise threshold (minimum 85% branch coverage).

Phase 4: Constructive Feedback & Iteration

• Categorize every comment using explicit prefix tags:
  • [BLOCKER]: Security vulnerability, data loss risk, or severe architectural violation requiring absolute fix prior to merge.
  • [NON-BLOCKER] or [SUGGESTION]: Stylistic preferences, refactoring options, or micro-optimizations that do not block approval.
  • [QUESTION]: Requests for clarification on design choices.
• Frame all feedback objectively, focusing on the code rather than the author (e.g., “Consider utilizing a prepared statement here to mitigate SQL injection vectors” vs. “You wrote vulnerable code”).
• Validate author responses and re-review code modifications iteratively until all [BLOCKER] items are fully resolved.

Phase 5: Final Sign-off & Merge Execution

• Issue final formal approval (Approve status in version control platform) only when all verification criteria are met.
• Ensure the author performs a clean squash-and-merge or rebase workflow to maintain a linear, intelligible Git commit history.
• Verify automatic branch deletion post-merge to maintain repository hygiene.

---

6. Quality Assurance & Verification Criteria

A code review is deemed compliant and verified only when it satisfies all structural benchmarks detailed below:

• SLA Compliance: Peer review initial response time must not exceed 4 business hours for standard priority tickets.
• Approval Thresholds:
  • Standard PRs require a minimum of two (2) explicit approvals from engineers qualified in the respective service domain.
  • Core infrastructure, IAM, or cryptography modifications require explicit sign-off from a Principal Systems Architect or Security Officer.
• Zero Critical Vulnerabilities: Static Application Security Testing (SAST) and Dynamic Application Security Testing (DAST) tools must report zero High or Critical severity findings.
• Test Delta Integrity: Overall repository test coverage must not decrease post-merge.

---

7. Practical Troubleshooting Scenarios

Scenario A: Stalemate on Architectural Style ([NON-BLOCKER] loops)

• Trigger: Author and Reviewer are locked in a subjective debate over variable naming, functional vs. imperative paradigms, or micro-patterns, delaying delivery past the SLA window.
• Immediate Corrective Action Protocol:
  1. The Reviewer must automatically downgrade the comment status to [SUGGESTION], prefixing it with an explicit note: "Non-blocking stylistic preference; deferring to author discretion."
  2. The Reviewer must clear the block and approve the PR if all functional, structural, and security criteria are met.
  3. Escalate the stylistic debate to the quarterly architecture guild meeting for global team standardization rather than blocking an active release.

Scenario B: Emergency Hotfix Bypass Attempt

• Trigger: An engineer attempts to merge a production hotfix directly without code review under the guise of an active P0 incident.
• Immediate Corrective Action Protocol:
  1. The version control system policy enforcement layer will automatically block direct pushes to protected branches (main, production).
  2. The Incident Commander (IC) must initiate the Emergency Break-Glass Protocol: open an expedited PR, page the designated On-Call Tech Lead for verbal/slack review, and secure a retroactive review ticket assigned within 24 hours of incident mitigation.
  3. Under no circumstances may automated security scanning gates be bypassed by individual contributors.

Scenario C: Unresponsive Reviewer / Abandoned PR

• Trigger: Assigned Reviewer fails to respond within 8 business hours, stalling a time-sensitive feature release.
• Immediate Corrective Action Protocol:
  1. The Author flags the bottleneck in the designated engineering operations channel (#eng-code-review-escalations).
  2. The rotation lead reassigns the review to the secondary on-call reviewer for that service domain within 30 minutes of escalation.
  3. If the original reviewer remains unresponsive without status declaration (e.g., PTO), a formal review strike is logged in the internal engineering metrics dashboard for operational review.