Code Review Guidelines Github

Standard Operating Procedure: Code Review Guidelines GitHub

1. Document Control & Revision History

---

2. Executive Summary & Scope

2.1 Clinical Statement of Operational Boundaries

This Standard Operating Procedure (SOP) defines the mandatory engineering controls, review mechanics, and governance criteria for all source code modifications submitted via GitHub pull requests (PRs). Adherence to this protocol is non-negotiable across all enterprise repositories to maintain system reliability, security posture, and compliance mandates (including SOC 2 Type II, ISO/IEC 27001, and HIPAA where applicable).

2.2 Scope

This document applies to all software engineers, systems architects, security analysts, and engineering managers authorized to commit code, review changes, or approve merges within the organization’s GitHub Enterprise Cloud organization. Any bypass of these procedures constitutes a direct policy violation and triggers an automated audit investigation.

---

3. Prerequisites & Tool Requirements

To execute code reviews and pull request operations within the enterprise environment, personnel and workstations must meet the following baseline requirements:

• Software & Tooling:
  • GitHub Enterprise Cloud account with mandatory Multi-Factor Authentication (MFA/WebAuthn) enforced.
  • Git command-line interface (CLI) version 2.40.0 or higher, configured with SSH keys or Personal Access Tokens (PATs) bound to enterprise SSO.
  • Integrated Development Environment (IDE) with enterprise-approved linters and formatters installed (e.g., VS Code, JetBrains Suite).
  • Command-line access to containerized runtime environments (Docker/Podman) for local reproduction testing when required.
• Security & Compliance Access:
  • Membership in the appropriate GitHub enterprise teams (e.g., @core-engineering, @sec-appsec).
  • Active status in the internal static application security testing (SAST) and software composition analysis (SCA) dashboards (e.g., Snyk, SonarQube, GitHub Advanced Security).
• Training & Certifications:
  • Completion of the Enterprise Secure Coding Practices (ESCP) certification module.
  • Passing grade on the Internal GitHub Workflow and Branch Protection Audit simulation.

---

4. Roles & Responsibilities (RACI Matrix Table)

Legend: R = Responsible, A = Accountable, C = Consulted, I = Informed.

---

5. Step-by-Step Operational Procedure

Phase 1: Pre-Submission Preparation (Author)

• 1.1 Verify that the local working branch is derived from the current main (or designated release) branch and has been rebased to eliminate merge conflicts.
• 1.2 Execute local unit tests, integration tests, and static analysis linters, confirming a 100% pass rate.
• 1.3 Ensure all changes strictly map to a single atomic user story or tracked bug ticket (Jira/Linear reference required in commit messages and PR title).

Phase 2: Pull Request Creation & Automation Gate (Author)

• 2.1 Open a pull request against the target protected branch using the standard enterprise PR template.
• 2.2 Populate all template sections: Summary, Technical Approach, Risk Analysis, and Verification Steps.
• 2.3 Verify that automated GitHub Actions CI/CD pipelines (Build, Test, Lint, SAST, SCA) initiate successfully and clear without error. Do not request manual reviews until automated gates pass.

Phase 3: Peer Review & Technical Inspection (Reviewer)

• 3.1 Review the diff with a focus on core architectural patterns, thread safety, memory management, and error handling.
• 3.2 Validate that no hardcoded secrets, API keys, or proprietary internal configurations are exposed in the code or commit history.
• 3.3 Verify that appropriate unit and integration test coverage is included for all newly introduced code paths.
• 3.4 Provide constructive, line-specific comments categorized by severity (e.g., [Blocking], [Non-Blocking], [Nitpick]).

Phase 4: Resolution & Re-Verification (Author & Reviewer)

• 4.1 Address all reviewer comments by committing targeted fixes or providing justified technical rebuttals.
• 4.2 Request re-review explicitly via the GitHub interface once all blocking comments are marked as resolved.
• 4.3 Confirm that all required approvals (minimum two distinct peer approvals, plus one security approval for core system services) are formally logged.

Phase 5: Merge Execution (Author / Release Manager)

• 5.1 Perform a final verification of branch protection status checks to ensure all gates remain green.
• 5.2 Execute the merge using the Squash and Merge strategy (unless otherwise specified by repository policy) to maintain a linear commit history.
• 5.3 Verify automatic branch deletion post-merge and confirm tracking boards update accordingly.

---

6. Quality Assurance & Verification Criteria

6.1 Audit Checklist

• Test Coverage Delta: Net new code must maintain or exceed 85% line and branch test coverage as measured by SonarQube.
• Vulnerability Thresholds: Zero High or Critical vulnerabilities permitted under Snyk/GitHub Advanced Security scans.
• Reviewer Independence: Reviewers must not be on the same immediate project reporting line as the author for critical security boundaries.
• Documentation Updates: Any modification to public APIs or operational interfaces must include corresponding updates to OpenAPI specs or technical wikis.

6.2 Acceptance Benchmarks

• Time-to-First-Review: $\le 4$ business hours from PR submission.
• Review Cycle Turnaround: $\le 24$ hours per iteration.
• Hotfix Exception Rate: $< 2%$ of total production deployments.

---

7. Practical Troubleshooting Scenarios

Scenario A: Automated SAST Gate Fails Due to a False Positive Vulnerability

• Symptom: GitHub Actions blocks the PR due to a flagged security vulnerability in a third-party dependency or safe algorithmic pattern, halting the merge pipeline.
• Immediate Corrective Action Protocol:
  1. Do not disable the SAST scanner or attempt to bypass the branch protection rule.
  2. Consult with the Application Security (@sec-appsec) team via internal secure channels, providing the explicit SAST rule ID and code context.
  3. If designated a valid false positive, the SecOps engineer applies a scoped suppression annotation directly in the code or via the dashboard, complete with an expiration date and mandatory justification comment.
  4. Re-run the CI/CD pipeline to verify gate clearance.

Scenario B: Unresolvable Merge Conflicts Due to Divergent Long-Lived Branches

• Symptom: A PR remains open for multiple days, resulting in extensive merge conflicts against main that break the standard GitHub web-interface merge utility.
• Immediate Corrective Action Protocol:
  1. Pull the latest remote main branch locally: git fetch origin main.
  2. Check out the feature branch and initiate an interactive rebase: git checkout feature-branch && git rebase origin/main.
  3. Resolve conflicts explicitly in the IDE, ensuring logical integrity is preserved across conflicting hunks.
  4. Continue the rebase: git rebase --continue.
  5. Force-push the rebased branch with lease to the remote repository: git push origin feature-branch --force-with-lease.
  6. Notify reviewers that the PR is ready for re-inspection of the resolved diff.

Scenario C: Reviewer Unresponsiveness Blocking Critical Production Hotfix

• Symptom: A critical production patch PR is stalled because assigned peer reviewers are unavailable, violating strict operational SLAs.
• Immediate Corrective Action Protocol:
  1. Author notifies the engineering escalation rotation lead via the designated emergency alerting channel (PagerDuty/Slack bridge).
  2. The escalation lead verifies the severity classification of the hotfix.
  3. The escalation lead reassigns the review to an active on-call engineer or utilizes administrative override privileges, provided an emergency post-incident review (PIR) ticket is simultaneously generated to log the protocol override.