Code Review Workflows: Moving Beyond “LGTM”
In high-performance engineering cultures, a Code Review (CR) is not a hurdle; it is the primary engine for knowledge distribution and quality assurance. Yet, many teams treat Pull Requests (PRs) as a bureaucratic formality, leading to the dreaded “Rubber Stamping” anti-pattern where reviewers simply comment “LGTM” (Looks Good To Me) without truly understanding the logic.
The Real-World Impact: On GitHub, your PR is your professional portfolio. A well-structured workflow ensures that security vulnerabilities are caught before they hit production, and that “bus factor” is minimized by ensuring at least two people understand every line of code. It’s the difference between a codebase that scales and a legacy nightmare that breaks every Friday afternoon.
The Anatomy of a Senior-Level Workflow
Expert developers don’t just push code; they curate an experience for the reviewer. This involves Atomic Commits—small, logical units of work—and Draft Pull Requests to signal work-in-progress. By integrating CI/CD checks (GitHub Actions) directly into the PR, the human reviewer is freed from checking syntax or linting and can focus on architectural soundness and edge cases.
Common Pitfalls to Avoid
- The Mega-PR: Submitting 2,000 lines of code across 50 files. No one can review this effectively. Break it down.
- Bikeshedding: Arguing over trivialities like variable names while ignoring a massive SQL injection vulnerability.
- The “Silent” Reviewer: Leaving a “Request Changes” without actionable feedback or a path to resolution.
Study Guide: GitHub Code Review Mastery
Code Review is the process of peer-evaluating source code to improve software quality and developer skills. In the GitHub ecosystem, this centers around the Pull Request (PR).
The Analogy: Think of a Code Review like a professional kitchen’s “Expeditor.” Before a plate goes to the customer, the Sous Chef checks the presentation, temperature, and seasoning. It’s not about doubting the cook; it’s about maintaining the restaurant’s Michelin star.
Core Concepts & Terminology
- Base vs. Head Branch: The
baseis where you want to merge (e.g.,main); theheadis your feature branch. - CODEOWNERS: A file that automatically assigns reviewers based on the file paths being changed.
- Status Checks: Automated tests or linters that must pass before a PR can be merged.
- Threaded Comments: Discussions linked to specific lines of code.
Security & Governance
Professional repos use Branch Protection Rules. Common settings include:
- Requiring a minimum number of approvals.
- Requiring signed commits (GPG/SSH).
- Restricting who can push to specific branches.
- Dismissing stale reviews when new commits are pushed.
Real-World Scenarios
Scenario 1: The Solo Developer
Context: Working on a personal project or a solo startup module.
Application: Even without a team, use PRs. Review your own code after a “cooling off” period (e.g., next morning). Use GitHub Actions to automate testing.
Why it works: It forces you to document “why” in the PR description, creating a searchable history for your future self.
Scenario 2: The Enterprise “Squad”
Context: 5-8 developers working on a microservice with strict uptime requirements.
Application: Mandate 2 approvals. Use CODEOWNERS to ensure a Senior Engineer or Security Lead reviews sensitive files (e.g., /auth or /billing).
Why it works: Distributes responsibility and prevents “accidental” merges of breaking changes into the trunk.
Interview Questions & Answers
- What is the difference between a “Merge,” “Squash and Merge,” and “Rebase and Merge”?
Merge keeps all history and creates a merge commit. Squash condenses all PR commits into one clean commit on the base branch. Rebase moves the feature commits onto the tip of the base branch, maintaining a linear history.
- How do you handle a “Review Loop” where a reviewer keeps asking for minor changes?
Request a brief sync-up (Zoom/Slack) to align on goals. Often, “nitpicks” can be addressed in a follow-up PR if they aren’t blockers for the current feature.
- What is a
CODEOWNERSfile?A file in the
.github/directory that defines which users or teams are responsible for specific directories or files in a repository. - Why would you use a “Draft Pull Request”?
To show work-in-progress, get early architectural feedback, and prevent accidental merging before the code is ready.
- How do you ensure PRs stay small?
Practice “Stacked PRs” or use feature flags. If a feature is large, merge the infrastructure first, then the logic, then the UI.
- What are “Protected Branches”?
Settings that prevent force-pushing, deleting branches, or merging without passing status checks and reviews.
- What is “Bikeshedding” in a code review?
Focusing on trivial, subjective details (like indentation) while ignoring high-level architectural or security flaws.
- How does GitHub Actions integrate with Code Reviews?
Actions act as the “First Reviewer,” running unit tests, linters, and security scanners (SAST) automatically on every push.
- What is the “Suggest Changes” feature in GitHub?
A feature allowing reviewers to provide specific code snippets that the PR author can apply with a single click.
- How do you handle sensitive data (API keys) found in a PR?
Close the PR immediately, invalidate the secret, and use tools like
git-filter-repoor BFG Repo-Cleaner to scrub the history before reopening.
Interview Tips & Golden Nuggets
- The Senior Mindset: When asked about tools, always pivot to process. Say: “I prefer Squash and Merge for a cleaner history, but the team’s consensus and the project’s scale dictate the final choice.”
- Trick Question: “Is a PR required for Git?” Answer: No, PRs are a GitHub/GitLab feature, not a core Git command. Git uses
git request-pullor patches via email. - Rebase vs. Merge: Know that Rebase creates a cleaner history but can be dangerous on public/shared branches because it rewrites commit SHAs.
- Empathy: Mention that code reviews are a social activity. Using “We” instead of “You” in comments (e.g., “Can we rename this?”) builds better team morale.
Comparison: Branching & Review Strategies
| Strategy | Use Case | Pros | Cons |
|---|---|---|---|
| GitHub Flow | Web Apps / CI/CD | Simple, fast, one main branch. | Harder to manage multiple versions. |
| Git Flow | Release-based software | Structured, handles ‘hotfixes’. | Complex; many long-lived branches. |
| Trunk-Based | High-velocity teams | Reduces merge conflicts. | Requires very high test coverage. |
The GitHub Review Pipeline
Ecosystem
- PR Templates: Standardize the info authors must provide.
- Labels: Use
bug,enhancement,critical.
Collaboration
- Review Requests: Tag specific teams.
- Discussions: Use for non-code architectural debate.
Automation
- Auto-merge: Enable for minor dependency updates.
- Linters: Catch style issues before the human sees it.
Decision Tree: Merge Strategies
- Is history cleanliness the top priority? → Use Squash and Merge.
- Need to see exactly when every feature was integrated? → Use Merge Commit.
- Maintaining a high-frequency, linear trunk? → Use Rebase and Merge.
CODEOWNERS combined with mandatory GitHub Actions security scanning. No code can be merged into main without a green scan from Snyk and an approval from the Security team if the /crypto directory is touched. This ensures speed for UI teams while maintaining a “Hardened Core.”