The problem only showed up when I checked which authentication middleware the agent had imported.

Our codebase had two: a v1 middleware backed by MongoDB and a v2 middleware backed by MySQL, which we had spent the previous quarter migrating.

New endpoints were supposed to use v2. The agent had used v1 for all three. Tests passed because user records still existed in both databases (that was the point of the migration), and the v1 middleware happily authenticated them. The code worked. But every new endpoint we shipped was reinforcing the legacy auth path we had just spent a quarter trying to retire.

I caught it on the second read. Twenty minutes after the comments, the engineer fixed it and reopened the PR. The third reviewer probably wouldn't have caught it. The migration timeline lived in a Slack thread from six months earlier. The rule that "new endpoints use v2" lived in my head.

This kind of catch is the slow-burn version of why AI changed my job as a tech lead. Code generation got faster. My review queue got longer. The hardest reviews were the ones where everything looked right, and the only thing wrong was something that lived in the team's collective memory rather than in the diff.

This handbook is about what we did to fix that. It's the story of how we went from drowning in clean-looking PRs to running a custom AI PR reviewer that catches a meaningful share of these mistakes before any human is pulled in. The fix turned out to be less about buying a better tool and more about moving the team's memory into a place the AI could actually read.

The lessons should transfer whether your team uses Claude Code, Cursor, Cline, GitHub Copilot, or any combination. The structure matters more than the tool.

Table of Contents

• The Old Bottleneck, and the One AI Created

• What the New Review Work Actually Looks Like

• Why I Did Not Just Buy a Tool

• The Realisation: Move the Rules Into the Codebase

• Two Files That Changed Everything: AGENTS.md and CLAUDE.md

• Where Per-Service Memory Files Earn Their Keep

• What This Looks Like on Disk

• Generated Documentation as a Side Effect

• Building the PR Review Command

• Guardrails: Read-Only by Default

• The Compounding Loop That Made the Real Difference

• Starting From Zero on an Existing Project

• What Still Needs Human Review

• A Two-Week Setup Plan

• What Is Working, What I Am Still Improving

• Sources

The Old Bottleneck, and the One AI Created

To understand why this fix was needed, it helps to remember what reviewing code looked like a couple of years ago.

Back then, the slow part was upstream of the PR. A ticket would land, and before anyone could open a branch, there was a long preamble of context-gathering.

Junior engineers needed time to understand what the change was for. Senior engineers had to explain business rules and architectural decisions. Tickets sat in "ready" columns for days while someone with the right context made themselves available. Then the writing itself took time, because typing real code is slower than typing comments about it.

That bottleneck mostly dissolved when the team got serious about AI-assisted development. Engineers used the agent to read the codebase, ask clarifying questions, draft an implementation plan, and produce a working branch in hours instead of days. Tickets moved through the queue faster. Junior engineers shipped more without blocking on senior availability. From the outside, this looked like an unambiguous win.

But the bottleneck didn't disappear. It moved.

Within a few weeks of widespread AI adoption, my review queue had doubled. Then tripled. Engineers were opening PRs faster than I could read them.

The PRs themselves looked clean: well-formatted, with sensible variable names, passing tests, and AI-generated descriptions that read better than most human-written ones.

On the surface, this was great. In practice, it was creating a different kind of pain. I was the senior engineer who knew which patterns mattered and which paths through the codebase were the right ones, and I was the bottleneck. The team's velocity was now capped by my reading speed.

The CircleCI 2026 State of Software Delivery report confirmed I was not alone. Drawing on more than 28 million CI workflow runs across over 22,000 organisations, the report showed feature branch throughput had grown 59% year over year, the largest jump CircleCI had ever measured. Main branch throughput, where code actually gets promoted to production, fell by 7% for the median team in the same period. Build success rates dropped to 70.8%, the lowest in five years.

The pattern was consistent across the industry. AI accelerated writing. The rest of the system absorbed the cost.

So the question for me, as a tech lead, became concrete: how do I unblock myself without lowering the bar?

What the New Review Work Actually Looks Like

Before I explain the fix, it helps to know what kinds of issues were actually piling up. They weren't the dramatic kind. None of them would crash production. They were small, recurring, and looked plausible at a glance.

Take the simplest case I kept catching. An engineer would ask the agent to add a delete button on a new screen. The button needed to call our existing backend delete endpoint. Instead of reusing the hook the team already had for that endpoint, the agent would write the fetch call inline.

The code worked. The tests passed. But a week later, when someone changed the backend response shape, only one of the two call sites got updated.

That kind of duplication doesn't show up in a code review unless the reviewer happens to remember that a hook exists.

Another example I saw constantly: the agent comparing a status field against the literal string "completed" instead of using the Status.Completed enum that the rest of the services used. The code ran. The tests ran. The next refactor of the enum quietly skipped the file. After a few days, someone would spend half a day debugging a state machine that was working fine until the agent's literal silently fell out of sync.

These were two-minute fixes once spotted, but spotting them took me a reasonable time per PR. The friction wasn't the difficulty. It was the repetition.

The pattern repeated across larger problems, too.

I once asked an agent to build an event creation wizard. The wizard needed several dropdowns and one new component.

We have a design system folder where shared UI components live, and the rule on the team is simple: check there first, and if you build something new, register it there.

The agent had no way to know that. It only loaded the wizard's own files, so it never opened the design system folder. It generated brand new dropdowns inline, with APIs that were almost identical to the ones we already had. The new component went straight into the wizard rather than into the design system. CI passed. The wizard worked. We caught the duplication in human review, but it was the kind of catch that depended entirely on a reviewer who happened to know the design system existed.

The same pattern hit in one of the repos I was looking at for backend architecture. Backend follows a strict four-layer pattern: route, controller, app, repo. Controllers must never call repository functions directly. That rule keeps authorisation centralised, business logic testable, and database concerns isolated.

One PR I reviewed had the agent calling repo functions straight from a controller, skipping the app layer entirely. The code worked. The tests passed because the agent had also written tests against the new shape. But it broke a discipline the team had spent years building. If that PR had landed, the next AI-assisted PR could have used it as a template, and the layering would have eroded one diff at a time.

The common thread is that all of these mistakes had something written down somewhere, in code, in a Slack thread, in a senior engineer's head, that would have prevented them. The information existed. The agent just couldn't see it.

Why I Did Not Just Buy a Tool

The obvious next move was to install one of the AI PR reviewers that flooded the market in 2026.

I evaluated several. Anthropic launched Claude Code Review in March 2026, billed on token usage and averaging \(15 to \)25 per review. CodeRabbit Pro charges \(24 per developer per month on annual billing, or \)30 per developer per month on monthly billing, with seats counted against developers who actually open PRs. Greptile in March 2026 moved to a base-plus-usage model at $30 per seat per month, including 50 reviews, after which each additional review costs a dollar. GitHub announced that all Copilot plans will transition to usage-based billing on June 1, 2026, with code reviews consuming both AI Credits and GitHub Actions minutes from that date.

For a small team with low PR volume, none of these is a dealbreaker. For a larger team running heavy AI-assisted development, the costs compound fast. A 10-person team running five PRs each per day blows through Greptile's included reviews in a single week. CodeRabbit Pro at \(24 per seat scales linearly with developers. The premium Claude Code Review at \)15 to $25 per PR is the most expensive option per review by an order of magnitude.

I looked at the cost numbers, but cost wasn't actually the deciding factor. The deciding factor was that none of these tools would have caught the problems I just listed.

A generic reviewer wouldn't have caught the v1/v2 middleware. It had no way to know v2 was the canonical path. A generic reviewer wouldn't have caught the duplicate dropdowns. It had no way to know our design system existed. A generic reviewer wouldn't have caught the bypassed architecture. It had no way to know that controllers must not call repositories.

The information that lets a reviewer flag any of these is exactly the information that lives in the team's head, not in any tool's default prompt.

The better-rated tools support custom rules, and that's where I started to see the real shape of the problem. Once you are configuring custom rules, you've already accepted that the value is in the rules. The tool is just whatever runs them.

This raised a different question: if the rules are the product, why pay per seat or per review for someone else's wrapper around them?

This is what made me change direction.

The Realisation: Move the Rules Into the Codebase

Once I started thinking of the rules as the product, the path forward got clearer.

I asked myself a simple question: what was I actually doing in code review that the AI was not? The answer turned out to be the same thing, over and over. I was typing review comments that captured a piece of the team's memory.

"Use the Status enum, not a string literal." "There is already a hook for this in /hooks/useDeleteItem." "Controllers must not import from the repo layer; route this through the app layer." "Check the design system folder before creating new components."

Each of those comments was knowledge that lived in my head and arrived in the codebase one PR comment at a time. None of it was available to the agent the next time it generated a similar PR.

So the fix was not to buy a smarter reviewer. The fix was to write the rules down in a place every agent on the team would read before any review happened.

If I had typed "use the enum, not a literal" three times in three different PRs, that was a rule the agent should know about from now on. If I had pointed at the design system folder for the fourth time, that was a rule. If I had explained the four-layer architecture twice in PR comments, that was a rule.

I needed somewhere to put these rules. That turned out to be a less obvious decision than I expected.

Two Files That Changed Everything: AGENTS.md and CLAUDE.md

If you start looking into how to give an AI agent a persistent project context, you run into two competing conventions almost immediately.

The first is AGENTS.md, an open standard that has gathered real momentum. According to InfoQ, by mid-2025, the format had already been adopted by more than 20,000 GitHub repositories and was being positioned as a complement to traditional documentation: machine-readable context that lives alongside human-facing files like README.md.

The standard's own site reports it is now used by more than 60,000 open-source projects and has moved to stewardship under the Agentic AI Foundation, which sits inside the Linux Foundation. The format is supported by OpenAI Codex, GitHub Copilot, Google Gemini, Cursor, and Windsurf, among others.

The second is CLAUDE.md, which is Anthropic's convention for Claude Code. The Claude Code documentation describes two complementary memory systems: CLAUDE.md, where you write the persistent context yourself, and an auto-memory mechanism that lets Claude save its own notes from corrections and observed patterns. By default, Claude Code reads CLAUDE.md, not AGENTS.md.

This split mattered for us because half the team uses Claude Code and the other half uses Cursor. We had two practical options: maintain both files with the same content (and accept the duplication), or symlink one filename to the other so both ecosystems read the same source of truth. We went with the symlink. It's one less thing to drift.

The next question was what to actually put in the file. After a few iterations, here's the shape that worked. Think of it as a briefing document for a new engineer who has read no code and seen no Slack threads. The minimum content was:

• The tech stack (languages, frameworks, package manager)

• The project structure, especially important for our monorepo

• Where shared utilities, components, and helpers live, and the rule that new code should reuse them before creating new versions

• Architectural patterns the project follows, with file path examples

• Anti-patterns and what to do instead

• Test conventions and where good examples live

• Pointers to deeper documentation when more detail is needed

Two practical rules emerged from the first month of using these files.

Keep them lean: There is a counterintuitive failure mode with long instruction lists: the agent doesn't just skip the new ones at the bottom. The average compliance across all of them drops. A bloated memory file becomes a memory file that the agent skims. If a section runs more than a paragraph or two, move it to a separate document and link to it.

Phrase rules as imperatives, not aspirations: "Controllers must not call repositories. Route through the app layer." beats "Try to keep controllers thin." The first is testable. The second is decorative.

That was the entry point. But a single root-level file was not enough for a monorepo with multiple services and frontends, which led to the next decision.

Where Per-Service Memory Files Earn Their Keep

A single AGENTS.md at the root of a monorepo collapses under its own weight pretty quickly. Each service in our codebase has its own architecture, conventions, and business rules. Trying to fit all of that into one file produced a long document that the agent treated as background noise, and we were back to the bloat problem from the previous section.

The pattern that worked: every service or app gets its own AGENTS.md at its root, and the project-level AGENTS.md becomes an index that points to them.

A per-service AGENTS.md covers things like:

• The architecture for this service (the four-layer pattern, the directory layout)

• Naming conventions specific to this service

• Test patterns and where good examples live

• Business rules that this service is responsible for

• Inter-service contracts and what other services consume from this one

• Pointers to deeper docs in docs/

• A "Lessons learned" section, which I'll come back to in the section on the compounding loop

The same lean rule applies. Keep it short, point at examples, and phrase guidance as imperatives.

The reason this works mechanically is that the agent loads the right files for the work at hand. When an engineer asks the agent to change something in backend/, the agent reads the project-level AGENTS.md, sees that work in backend/ should be guided by backend/AGENTS.md, and loads that file. It doesn't load the frontend's AGENTS.md, because that work is somewhere else. The context window stays focused on what's relevant.

Without this split, you have two bad options. Either you put everything in the root file, where the agent ignores most of it, or you put nothing in the root file, where the agent has no team context at all. The per-service split gives you both depth and signal.

But these files only work if the deeper docs they point to actually exist, which is where the next piece of the system came in.

What This Looks Like on Disk

Before going further, it helps to see the whole structure laid out. Here's the shape we settled on for our monorepo. The exact folder names follow Claude Code's conventions. If you use Cursor, it would be .cursor/, and if you use Cline, it would be .clinerules – but the shape transfers directly.

project-root/
├── AGENTS.md                       # symlink to CLAUDE.md
├── CLAUDE.md                       # root memory file
├── README.md                       # human-facing project readme
│
├── .claude/                        # tool-specific config folder
│   ├── README.md                   # explains the .claude/ layout
│   ├── settings.json               # permissions and guardrails
│   ├── agents/                     # specialised subagents (optional)
│   ├── commands/                   # slash commands engineers run
│   │   ├── review-pr.md            # the PR review command
│   │   └── plan-feature.md         # implementation plan command
│   ├── hooks/                      # lifecycle hooks (optional)
│   ├── pr-rules/                   # rule files for PR review
│   │   ├── common.md               # rules that apply to every PR
│   │   ├── frontend.md             # rules for frontend changes
│   │   ├── backend.md              # rules for backend changes
│   │   ├── service-a.md            # rules for service-a
│   │   └── service-b.md            # rules for service-b
│   └── skills/                     # reusable workflows
│
├── frontend/
│   ├── AGENTS.md                   # frontend conventions
│   ├── docs/
│   │   ├── overview.md
│   │   ├── architecture.md         # routing, state, data layer
│   │   ├── design-system.md        # design system reference
│   │   └── testing.md              # test conventions
│   └── src/
│
├── backend/
│   ├── AGENTS.md                   # the four-layer pattern
│   ├── docs/
│   │   ├── overview.md
│   │   ├── architecture.md         # route -> controller -> app -> repo
│   │   ├── auth.md                 # v1 vs v2 middleware
│   │   ├── business-rules.md
│   │   └── integrations.md
│   └── src/
│
├── service-a/
│   ├── AGENTS.md
│   ├── docs/
│   │   ├── overview.md
│   │   ├── business-rules.md
│   │   └── integrations.md
│   └── src/
│
└── service-b/
    ├── AGENTS.md
    ├── docs/
    │   ├── overview.md
    │   ├── business-rules.md
    │   └── integrations.md
    └── src/

A few things worth pointing out:

The .claude/ folder uses standard subfolder names: commands, agents, hooks, skills. These follow Claude Code's plugin model, but most modern AI coding tools have similar slots. Following the conventions makes the structure recognisable to anyone on the team and lowers the cost of switching tools later.

The pr-rules/ folder isn't a standard convention. It's a folder we created to hold per-area review rules that the PR review command loads selectively. You don't have to call it pr-rules – the name matters less than having one place where review rules live.

Each service has its own AGENTS.md plus a docs/ folder. The root AGENTS.md is short and acts as an index. It tells the agent things like "if you touch files in backend/, also read backend/AGENTS.md first." The per-service file then points at the deeper docs as needed.

Generated Documentation as a Side Effect

Setting up per-service AGENTS.md files surfaced a problem I had been quietly avoiding. Most of our services didn't have decent documentation. Not API reference material, which lives in code, but the higher-level "what does this service do, what business rules does it enforce, what does it consume and produce" information that lives in nobody's head except the original author's.

The honest reason was that writing this kind of documentation by hand had never paid back the time it took. By the time the doc was finished, half of it was already stale.

So I tried something I wouldn't have considered earlier. I used the AI itself to generate a first draft for each service. I pointed the agent at each service's code and asked it to produce a docs/ folder with a specific structure: an overview, a list of business rules, an integrations document, a domain model, and any quirks worth knowing. The agent read the code, traced the call paths, and wrote a draft.

I then reviewed the output by hand, corrected the things it got wrong, and committed the result. The first drafts were 70-80% correct. The remaining 20-30% was where the agent had made plausible but wrong inferences, and those were exactly the cases where human review mattered.

The generated docs ended up serving two audiences. The agent uses them when reasoning about changes, which means it has real context for the service it's touching rather than guessing from local files. And new engineers use them on their first day, which has cut our onboarding time noticeably.

We used to write onboarding documents that drifted out of date within months. These docs stay closer to current because the agent reads them on every PR, and any drift gets surfaced when the agent gives wrong advice based on stale information.

The pattern that works is to keep the per-service AGENTS.md short and pointing at the docs, rather than duplicating their content. AGENTS.md is the always-loaded index. docs/ holds the details. The agent loads the relevant doc on demand when the task calls for it.

With the rules in place and the docs in place, I had everything I needed to build the actual reviewer.

Building the PR Review Command

This is the piece that most directly unblocked my queue.

This command didn't appear out of nowhere. It started as the checklist I was running through in my head every time I opened a PR. I was reviewing every change manually, leaving the same comments, flagging the same patterns. So I wrote that checklist down, expanded it with references to the per-service docs for the harder rules, and turned it into a command anyone on the team could run.

Then I handed it to the engineers and changed the rule: run this on your own branch before marking the PR ready for review. That single shift moved the work from after the PR was opened to before. Engineers now catch 90-95% of the blockers, improvements, and nice-to-haves on their own machine, fix them locally, and only then push the change.

The PR description includes the AI's summary, so when anyone opens the PR, they can see the reviewer's green signal at the top before even reading the diff.

GitHub stays clean. The conversation on the PR becomes about the things that actually need a human, not the recurring stuff the team already knows how to fix.

The command lives in .claude/commands/review-pr.md. Here's a generalised version. Your tool's command structure may differ, but the shape is what matters.

# Review PR

Review the current branch's PR. Be direct. Cite `file:line`. Surface real issues,
no padding.

## 1. Scope the diff

Run, in order:

    gh pr view --json number,title,body,headRefName 2>/dev/null || true
    git fetch origin main
    git log --no-merges origin/main..HEAD --oneline
    git diff origin/main...HEAD --stat
    git diff origin/main...HEAD

Read the PR body. Note the stated intent. Every change should trace to it. Flag
anything that does not.

Use `...` (three dots) for the diff. It compares against the merge base and
excludes commits brought in by merging main.

## 2. Load rules

Always read `.claude/pr-rules/common.md`.

Then read the per-area file for each workspace touched in the diff:

| Workspace path | Rules file                      |
| -------------- | ------------------------------- |
| `frontend/**`  | `.claude/pr-rules/frontend.md`  |
| `backend/**`   | `.claude/pr-rules/backend.md`   |
| `service-a/**` | `.claude/pr-rules/service-a.md` |
| `service-b/**` | `.claude/pr-rules/service-b.md` |

For non-trivial changes, follow doc pointers inside the rules files (for
example, `backend/AGENTS.md`, `backend/docs/architecture.md`).

Apply every entry under each file's "Lessons learned" section as a check.

## 3. Output

Use exactly this format.

    ## Summary
    <one paragraph: what the PR does, whether it matches the stated intent>

    ## Blocking
    - [file:line] issue, why it blocks

    ## Should fix
    - [file:line] issue

    ## Nice to have
    - issue

    ## Verified
    - what was checked and looks good

If nothing blocks, say so. Do not manufacture concerns.

If you find an issue worth remembering for future PRs, suggest the bullet to
add to the relevant rules file's "Lessons learned" section. Do not edit the
rules file yourself, leave that to the human.

A few of the design choices in this command turned out to matter more than I expected.

The structured output format (Summary, Blocking, Should fix, Nice to have, Verified) keeps the review easy to scan and easy to paste into a PR description. The "Verified" section is the most underrated of the five: it tells the human reviewer what the AI already checked, so they can spend their attention elsewhere. Without it, the human reviewer ends up doing the same checks twice.

The instruction to be direct and stop padding does real work. Without it, AI reviewers tend to manufacture concerns to look thorough, which trains engineers to skim past the bot. Telling it explicitly to say "nothing blocks" when nothing blocks made the signal-to-noise ratio of the output much better.

The "suggest a bullet for the rules file" instruction at the end is the heart of the whole system, and I'll explain why in the section on the compounding loop. The key constraint here is that the agent suggests the bullet but doesn't commit to it. A human evaluates whether it's general enough to be a rule, and only then adds it to the file. That manual step is what keeps the rules sharp instead of bloated.

With each PR, if humans fix something or the AI suggests something, you keep adding those to your MD files and keep improving your agents for the future. The result compounds quickly.

One more thing here: the diff-scoping commands are all read-only. The command shouldn't be able to push, edit PRs, or close anything. Which is the next piece of the system.

Guardrails: Read-Only by Default

Giving an AI agent broad permissions on your codebase is a security incident waiting to happen. Even if you trust the model to behave, an LLM occasionally does unexpected things, and a fast-moving agent on an unrestricted shell can cause damage in seconds.

The fix is a settings.json (in Claude Code – other tools have their own equivalents) at the root of .claude/ that explicitly declares what the agent can and can't do. The deny list matters more than the allow list, and a good one is organised around four categories of risk.

The first is secrets and configuration. Any read against anything that appears to be a credential is blocked. That covers .env files of every variant (.env, .env.local, .env.production, .env.test, and so on), .npmrc, .netrc, .pgpass, id_rsa, id_ed25519, *.pem, *.key, *.p12, **/credentials.json, **/secrets.json, **/.aws/**, **/.ssh/**, **/.gcloud/**, and **/.kube/**. Environment dumps are blocked too: env, printenv, set, export. The agent has no legitimate reason to read or echo any of these, ever.

The second is destructive Git operations. The agent can read Git history but can't rewrite or push it. Blocked: git push, git commit, git revert, git cherry-pick, git merge, git rebase, git reset --hard, git tag. Allowed: git fetch, git status, git log, git diff, git show, git branch, git rev-parse, git merge-base, git config --get.

The third is write operations on PRs and issues. The agent can read your GitHub state but can't act on it. Blocked: gh pr create, gh pr edit, gh pr merge, gh pr close, gh pr comment, gh pr review, gh issue create, gh issue edit, gh issue close, gh issue comment, gh release create, gh repo create, gh repo edit, gh repo delete. Allowed: gh pr view, gh pr list, gh pr diff, gh pr checks, gh issue view, gh issue list, gh release view.

The fourth is workflow and automation control. These are the surfaces where a compromised or misled agent could do the most damage. Blocked: gh workflow run, gh run rerun, gh run cancel, gh secret, gh variable, gh auth, gh ssh-key, gh gpg-key, and the unrestricted gh api.

For shell commands the agent legitimately needs to run, like build and test commands, allowlist specific patterns: pnpm test, pnpm lint, pnpm format:check, pnpm build, pnpm vitest. Anything outside the allowed list requires human confirmation. These are your own settings – I've just mentioned what I prefer.

The pattern is simple: read-only by default, write-allowed only for the specific commands you have explicitly approved. The agent can investigate, plan, and recommend. It can't ship.

With the structure in place and the guardrails set, the system started doing its job. What I didn't expect was how much better it would get over the months that followed.

The Compounding Loop That Made the Real Difference

When we started, the AI reviewer was useful but not transformative. It caught some obvious issues, missed plenty of subtle ones, and produced a fair amount of noise.

The first month, my review burden dropped by 35%. The time I was spending on PR checking was reduced to 1/3, almost. Decent, not life-changing.

What changed over time wasn't the tool. It was the rules.

Every time a PR creator and reviewer caught something the AI had missed, we were adding bullets to the relevant rules file. Every time the AI flagged something useful that turned out to be a recurring pattern, the agent's own suggestion at the end of the review went into the file.

After a few days, the rules files had grown into something that captured a meaningful fraction of the team's collective review knowledge, written down in a place every agent on the team would read.

The catch rate went up. The noise went down because the rules also said what was acceptable and what we already considered solved. New engineers stopped getting the same comments on their first three PRs because the AI caught the comments first. Engineers joining the team didn't have to absorb the conventions through six months of review feedback. They installed the project, opened it in their editor, and the agent already knew.

This is the part most teams miss when they evaluate AI PR review tools. They look at the catch rate today and decide whether the tool is worth the price. The catch rate today isn't the right number. The right number is what the catch rate looks like in six months, after the rules file has absorbed every recurring mistake your team has made.

A single rule written down today saves a small amount of review time. Over a hundred PRs, it saves more. After a year, the rules file is a written-down version of a tech lead's accumulated taste. We've switched between Claude Code, the GitHub Copilot CLI, and Cursor for various tasks during this period. The AI tool changes, but the rules file in the repo stays the same.

The discipline that makes this work is treating the rules file as living documentation. Every recurring review comment is a candidate for promotion into the file. If you catch yourself typing the same feedback in two different PRs, that's a rule that belongs in pr-rules/. The "suggest a bullet" instruction in the review command is what makes this practical: the AI does the typing, the human does the deciding.

This is also what made me realise the system was worth the time it took to set up. The PR review command, on its own, is useful but unremarkable. The compounding loop is what turns it into infrastructure.

Starting From Zero on an Existing Project

If you've read this far and feel like the gap between your project and what I just described is a sprint of work, that's the most common reaction. It's also not correct.

The blank AGENTS.md is intimidating, especially on an existing codebase. You know your team has a thousand conventions, and writing a thousand rules sounds like a project that takes weeks before it produces any value.

The honest answer is that you can't write all the rules up front, and you shouldn't try. The first version of any of these files should take an afternoon, not a sprint.

Here's how I would actually start.

Run /init (or your tool's equivalent). In Claude Code, /init scans the project, infers the obvious shape (language, framework, entry points, build commands), and writes an initial CLAUDE.md. The output is a starting point, not a finished file. Read it, delete most of what it generates, and keep the bones.

Then add three things, each one bullet long.

First, an architecture rule. Pick the single most important convention your team enforces. For us, that was the four-layer pattern. The bullet was: "Controllers must not call repository functions directly. They must go through the app layer."

Second, a discoverability rule. Pick the single most important shared resource the team has, the one new code is most likely to duplicate. For us, that was the design system. The bullet was: "Before creating a new UI component, check /src/design-system/ first."

Third, a "do not touch" rule. Pick the single most dangerous file or area in the codebase. Auth, billing, or migrations whichever has the most production risk. The bullet was: "Do not modify files in /auth/ without human approval."

That's enough to start. Three rules, ten minutes of writing, and most of your team's recurring AI mistakes start to drop.

If even three rules feels like too much, start with one. Pick a single line that matters in your codebase and write it down.

"No any types in TypeScript." "Always use the enum, never compare against the string literal." "Run the linter before opening a PR." It doesn't have to be sophisticated. It doesn't have to cover edge cases. It just has to capture one piece of judgement that lives in your head today and would otherwise stay there.

Tomorrow, add another. The first week, you might catch 5% of the recurring mistakes. By 20 or 30 PRs in, you might catch 20-30%. The rules file doesn't need to be impressive on day one. It needs to exist and keep growing.

This is the compounding effect I'll come back to soon, and it's the reason this approach works on real projects rather than just in theory.

From there, the file grows the same way it would grow for any team. Every review catch becomes a candidate rule. After a few weeks, you have ten or fifteen rules. After a few months, you have a real review system.

The mistake is trying to write the perfect file on day one. The right file is the one you start with and keep editing.

What Still Needs Human Review

This system doesn't replace human review, and it shouldn't be allowed to.

The AI reviewer catches what the rules describe, plus a fair number of obvious things it would have spotted anyway. It doesn't catch problems that depend on context the rules don't capture. It doesn't catch product judgement. It doesn't catch the question of whether the change should have been built at all.

It also has an important blind spot when reviewing AI-authored code. The reviewer shares the same training data and reasoning patterns as the agent that wrote the code. If the original agent missed the v1/v2 distinction because it had no way to see the migration timeline, an AI reviewer reading the same diff has the same problem. Two AIs in a review loop are not two independent reviewers. They share blind spots.

That is why the AI reviewer in this setup never approves a PR. It produces a structured review that goes into the PR description. A human still reads the change and approves it. The AI is the first pass, not the gate.

Accountability also has to live with a human. When something the AI approved breaks production, someone has to own the post-mortem and decide what changes are needed for next time. The AI can't be that person. What it can do, well, is reduce the stack of small mistakes a human reviewer has to find before they get to the harder questions.

A Two-Week Setup Plan

If you want to set this up for your own team, here's a concrete plan that fits in a couple of weeks. None of this needs to happen in a single push.

Day 1: Bootstrap the memory file.

Run /init (or your tool's equivalent) at the root of the project. Read the generated CLAUDE.md (or AGENTS.md). Delete most of it. Keep the tech stack and project structure sections.

Add the three rules from the previous section: one architecture rule, one discoverability rule, and one "do not touch" rule. Decide whether you want both files or a symlink.

Day 2: Add per-service files for your highest-risk areas

Pick the two or three areas of the codebase that change most often or carry the most risk. Add an AGENTS.md to each, following the same lean pattern. Include the architectural pattern for that area, the naming conventions, where to find good test examples, and pointers to any existing docs. Skip anything that doesn't need to be there yet.

Day 3: Set up the directory structure and guardrails

Create a .claude/ folder (or your tool's equivalent) at the root, with commands/ and pr-rules/ subfolders. Add a settings.json with the deny list categories from the guardrails section. Test that the agent can't read a .env file, run git push, or create a PR. If any of those work, fix the settings before doing anything else.

Day 4: Write the PR review command

Adapt the command in this article to your structure. Include the diff scoping, the rule loading, the output format, and the "suggest a new rule" instruction at the end. Run it on a branch you've already merged, and tune the output until it's useful.

Day 5: Run it on real PRs

Have one or two engineers run the command on their next PRs before opening them. Read the output. Note what it caught, what it missed, and what was noise. Add the missing catches to the rules files. The first week is mostly tuning.

Week 2: Roll out and document

Once the command produces useful output reliably, ask the whole team to run it before opening PRs and paste the output into the PR description. Add a short section to your contributing guide explaining the workflow. Set a recurring item in your team's rituals to review the rules files monthly and trim anything that has gone stale.

That gets you to a working system. From there, the maintenance is incremental. Every recurring review comment becomes a candidate rule. Every architectural decision becomes a candidate update to the relevant AGENTS.md. The system improves as a side effect of the work the team is already doing.

What Is Working, What I Am Still Improving

Here's my honest assessment after a few months of running this:

What's Working

My review burden is meaningfully smaller. Engineers fix most of the easy mistakes before I see the PR. The "Verified" section of the AI's output tells me what to skip past. New engineers ramp faster because the conventions live in a place their tooling reads. The rules files have grown into something I would actually use to onboard someone new.

What Isn't Finished

The AI still misses problems that depend on context, and the rules don't capture them. The rules files grow, but they also need pruning, and we haven't been disciplined about that.

We're still figuring out how to handle rules that apply only conditionally. Docs are helping in that case, but we need to keep those up to date. And no system survives a determined engineer who skips the workflow or docs when they're in a rush.

There's no shortcut here. The work is real, ongoing, and mostly about discipline. The discipline is treating your codebase as something the AI needs to learn, and treating every recurring review comment as something that should be written down once instead of typed thirty times. If you're willing to do that, the tools take care of the rest.

If you take three things from this article, take these.

1. First, don't pay for a generic reviewer to do a job your codebase needs to inform. Generic reviewers catch generic problems. Most of your real review work is specific to your team.

2. Second, put the rules in a file the AI reads, not in your head. AGENTS.md, CLAUDE.md, per-service files, per-area rules files. Pick a structure and stick to it.

3. Third, treat every human review catch as a chance to update the rules. The compounding effect over months is the entire point. A review system that improves itself is worth more than any single tool.

That's the system. It took a couple of weeks to build the foundation and a few months for the rules to mature. It costs very little to run, and it has done more for our PR throughput than any tool I evaluated.

Sources

• CircleCI's 2026 State of Software Delivery report, analysing more than 28 million CI workflows from over 22,000 organisations: https://circleci.com/resources/2026-state-of-software-delivery/

• CircleCI's blog post detailing the year-over-year throughput numbers, including the 59% feature branch growth and the main branch decline: https://circleci.com/blog/five-takeaways-2026-software-delivery-report/

• GitHub announcement of Copilot's transition to usage-based billing on June 1, 2026: https://github.blog/news-insights/company-news/github-copilot-is-moving-to-usage-based-billing/

• GitHub changelog confirming Copilot code review will start consuming GitHub Actions minutes on June 1, 2026: https://github.blog/changelog/2026-04-27-github-copilot-code-review-will-start-consuming-github-actions-minutes-on-june-1-2026/

• AGENTS.md, the open standard's official site, including its stewardship under the Agentic AI Foundation and the Linux Foundation: https://agents.md/

• Anthropic's Claude Code documentation on the memory system, including CLAUDE.md, auto memory, and the /init command: https://code.claude.com/docs/en/memory

• Anthropic's Claude Code GitHub Actions documentation, including notes on token-based billing and recommended cost controls: https://code.claude.com/docs/en/github-actions

• CodeRabbit's pricing documentation, confirming the per-developer-per-month seat model: https://docs.coderabbit.ai/management/plans

• Greptile's March 2026 pricing announcement, introducing the base-plus-usage model at $30 per seat per month with 50 included reviews: https://www.greptile.com/blog/greptile-v4

• HumanLayer's write-up on writing a good CLAUDE.md, including data on instruction-following degradation: https://www.humanlayer.dev/blog/writing-a-good-claude-md

And managers frequently view refactoring as "not a business need" until it boils over and becomes the most significant business need possible.

"Oh, our software somehow works. We can't implement any new changes. And oh, everyone is quitting because work is miserable."

In this article, I’ll walk you through the steps I use to refactor a complex codebase. We’ll talk about setting goals, writing tests, breaking up monoliths into smaller modules, verifying changes, making sure existing features still work, and keeping tabs on performance. I’ll also show you how to speed up reviews using AI tools.

By following these steps, you can turn complex, fragile code into a clean, reliable codebase your team can own.

The Issue of Technical Debt

As projects grow and evolve, technical debt increases. Code that was once functional and manageable turns into an unmaintainable mess, where even small changes become risky and time-consuming.

Despite the obvious need for cleanup, refactoring rarely gets prioritized because there's always something more urgent, new features, bug fixes, and client demands.

I’ve had conversations with engineers, many of whom are working on enterprise software and are fully aware of their codebase's code smells and inconsistencies. They dislike the situation but feel powerless to change it.

So how do we shift from a culture of writing for pure functionality to a culture that values maintainability, especially for complex codebases?

It’s usually a mistake to completely halt new feature development for a long refactoring period (except perhaps in emergencies). Business needs still exist, and putting everything on hold can create tension and lost opportunities. It’s better to find a balance so you’re still delivering value to users even as you clean under the hood.

While there is no one-size-fits-all solution, a structured approach can help teams introduce sustainable refactoring practices, even in environments where management is resistant. Let’s explore how this works.

Table of Contents:

• What is Refactoring?

• Preparing for Refactoring

  • Secure Management Buy-in

  • Ensure a Safety Net with Automated Testing

  • Identify High-Risk Areas

  • Set Clear Refactoring Goals

• Techniques for Refactoring Complex Codebases

  • 1. Identifying and Isolating Problem Areas

  • 2. Incremental vs. Big Bang Refactoring

  • 3. Breaking Down Monolithic Code

  • 4. Ensuring Backward Compatibility

  • 5. Handling Dependencies and Tight Coupling

  • 6. Testing Strategies (Safely Refactoring with Confidence)

  • 7. Refactoring Without Breaking Performance

  • 8. Automate Code Reviews with AI Tools

  • Summary

What is Refactoring?

Many people all too often use the word "refactor" when they mean a targeted rewrite.

As Martin Fowler famously said,

“Refactoring is a controlled technique for improving the design of an existing code base. Its essence is applying a series of small behavior-preserving transformations... However, the cumulative effect... is quite significant.”​

In practice, this means continuously polishing code to reduce complexity and technical debt.

While traditional software development follows a linear approach of designing first and coding second, real-world projects often evolve in ways that lead to structural decay. Refactoring counteracts this by continuously refining the codebase, transforming disorganized or inefficient implementations into well-structured, maintainable solutions.

A targeted rewrite is a focused overhaul of a specific aspect of an application, often affecting multiple parts of the codebase. It carries more risk than refactoring but is still controlled and contained.

Preparing for Refactoring

Even the most skilled refactoring effort can stall without proper preparation. Before you start moving code around, laying a foundation that will keep your work organized and your team on the same page is crucial.

Here are some steps you can take to ensure your refactoring efforts are successful.

Secure Management Buy-in

As I’ve already discussed, getting time for refactoring can be difficult in feature-driven organizations. Often, management will accept refactoring investment if you can tie it to business outcomes, faster time to market, fewer outages (which translates to happier customers), and the ability to take on new initiatives.

Make those connections explicit. For example, you could say:

“If we refactor our reporting engine now, it will make it feasible to add the analytics module next quarter, which unlocks a new revenue stream.”

Or use data:

“We spent 30% of our last sprint fixing bugs in module Y. After refactoring Y, we expect that to drop significantly, freeing time for new features.”

Business-minded arguments help justify the balance.

Ensure a Safety Net with Automated Testing

As you refactor, tests are your safety net. Before modifying a component, write characterization tests around it if they don’t exist.

# example: characterization test for a legacy function
def legacy_calculate_discount(price, rate):
    # ... complex logic you don't fully understand yet ...
    return price * (1 - rate/100) if rate < 100 else 0

def test_legacy_calculate_discount():
    # capture existing behavior
    assert legacy_calculate_discount(100, 10) == 90
    assert legacy_calculate_discount(50, 200) == 0

These tests capture the current behavior, so you’ll know if you accidentally change it. Unit tests, integration tests, and e2e tests all validate that refactoring hasn’t broken anything.

It’s often worth investing time in setting up a continuous integration pipeline so that every change triggers automated tests. This gives rapid feedback and confidence that you’re not introducing regressions. Robust testing and CI/CD enable you to move faster and refactor with peace of mind.

# .github/workflows/ci.yml
name: CI
on: [push, pull_request]
jobs:
  test:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-python@v4
        with: python-version: '3.10'
      - run: pip install -r requirements.txt
      - run: pytest --maxfail=1 --disable-warnings -q

Identify High-Risk Areas

The first step is to figure out what to refactor. High-risk areas are parts of the code likely to cause bugs or slow development. Common signs include long methods, large classes, duplicate code, and complex conditional logic​.

Such code “smells” often hint at deeper design problems. Tools like static analysis can automatically flag these issues.

For example, SonarQube will mark code smells (like high complexity or long methods) that increase technical debt​. Using SonarQube or similar tools, you can generate reports on code complexity (for example, cyclomatic complexity metrics​) and find hotspots in the codebase that need more attention.

Set Clear Refactoring Goals

Before refactoring code, define the goal.

Goals must be specific and measurable. For example, you might aim to reduce a class’s size or a function’s cyclomatic complexity by a certain amount or to increase unit test coverage from 60% to 90%.

Each goal is tied to a measurable outcome: shorter methods, fewer if statements or classes with a single responsibility, faster execution for processing orders, higher test coverage, and no unused code. These targets will guide our refactoring plan and let us verify when we’ve succeeded.

Tip: Write down your refactoring goals and share them with your team. This sets expectations that you’re not adding new features in this effort, just making the code cleaner and more robust. It also helps justify the time spent by showing the benefits (like more straightforward future additions and fewer bugs).

Techniques for Refactoring Complex Codebases

1. Identifying and Isolating Problem Areas

It can be overwhelming to decide where to start refactoring a large codebase. Not every part of the code needs refactoring – some areas are delicate or rarely touched.

The most impactful refactoring efforts typically target the “problem areas”: parts of the codebase that are overly complex, error-prone, or act as bottlenecks for development and performance. Identifying these areas is a crucial first step.

Techniques for Finding Hotspots

Team knowledge & developer frustration

Don’t underestimate the value of anecdotal information from the team. Which parts of the code do developers dread working in? Often, the team’s instincts point to areas that are hard to understand or modify (for example, “the accounting module is a black box, we hate touching it”). These could be areas to improve.

In my experience, simply asking, “If you had a magic wand, which part of the code would you rewrite?” yields very insightful answers.

Code complexity metrics

Use static analysis tools to measure cyclomatic complexity, code duplication, large functions/classes, and so on. Files or modules with extremely high complexity numbers or thousands of lines are good candidates for scrutiny. But static complexity alone doesn’t tell the whole story – a file might be ugly but rarely touched.

Change frequency (Churn)

Look at version control history to see which files are often changed, especially those associated with bug fixes or incidents.

Hotspot analysis

A robust approach combines complexity and change frequency to find “hotspots.” For example, a tool or technique plotting modules by their complexity and how often they change can highlight the problematic areas. CodeScene (a code analysis tool) popularized this: hotspots are parts of the code that are highly complex and frequently modified, indicating areas where “paying down debt has a real impact”​.

If a module is a mess and developers are in it every week, improving that module will likely yield outsized benefits (fewer bugs, faster adds).

Performance bottlenecks and crashes

Some parts of the codebase become targets for refactoring because they cause frequent performance problems or outages. For instance, if a specific service or job crashes often or can’t keep up with the load, you might need to refactor it for stability.

How to Isolate Problem Areas

Once you’ve identified a hotspot or problem area, the next challenge is isolating it so you can refactor safely. In a complex system, nothing lives in complete isolation. That problematic module likely interacts with many others.

Here are strategies to isolate and tackle it:

Break dependencies (Create seams)

Michael Feathers (in Working Effectively with Legacy Code) introduced the concept of “seams” – places where you can cut into a codebase to isolate a part for testing or refactoring. This might mean introducing an interface or abstraction between components so you can work on one side independently.

For example, suppose PaymentService is tightly coupled to StripeGateway, with direct calls scattered throughout the code.

# payment_service.py

def charge_customer(order_id, amount):
    # Hardcoded dependency to Stripe
    stripe = StripeGateway()
    stripe.charge(order_id, amount)

To isolate and refactor the payment logic safely, you can introduce a PaymentProcessor interface and have PaymentService depend on that interface instead. Then, create an adapter like StripeAdapter that implements PaymentProcessor and delegates to the existing Stripe logic.

This way, you can safely refactor or even replace the Stripe integration behind the StripeAdapter without impacting PaymentService or any other module that uses it. As long as the PaymentProcessor interface is honored, the rest of the system remains unaffected.

# interfaces.py

class PaymentProcessor:
    def charge(self, order_id, amount):
        raise NotImplementedError


# stripe_adapter.py

class StripeAdapter(PaymentProcessor):
    def charge(self, order_id, amount):
        # Internally still uses Stripe
        stripe = StripeGateway()
        stripe.charge(order_id, amount)


# payment_service.py (Refactored)

class PaymentService:
    def __init__(self, processor: PaymentProcessor):
        self.processor = processor

    def charge_customer(self, order_id, amount):
        self.processor.charge(order_id, amount)

“Branch-by-abstraction”

This technique is related to the above and is often used in continuous delivery. The idea is to add a layer of abstraction (like an interface or proxy) in front of the old code, have both old and new code implementations behind it, and then gradually shift usage from the old to the new implementation. For a while, you might have a temporary state where both versions exist (perhaps toggled by a config or feature flag).

This is similar to how the strangler fig pattern works at an architectural level. It’s a bit of extra work (since you maintain two paths for a while), but it allows you to migrate functionality and fall back if needed incrementally.

Aim to identify the 20% of the code causing 80% of the problems. Focus your refactoring energy there for maximum impact. When you do, create a plan to isolate that area via abstractions, interfaces, modules, or other means so that you can work on it with minimal risk of side effects. The more you can contain the blast radius of a refactoring, the more confidently you can move forward.

2. Incremental vs. Big Bang Refactoring

One of the first strategic decisions is approaching the refactor incrementally or going for a “big bang” overhaul. In most cases, an incremental approach is preferable, but there are scenarios where more significant coordinated refactoring steps are considered.

Let’s break down what these mean:

# before: one large function with multiple responsibilities
def process_order(order):
    validate(order)
    apply_discount(order)
    save_to_db(order)
    send_confirmation(order)
    log_metrics(order)
    update_loyalty_points(order)
    # potentially more steps 

# after: refactored incrementally into clearer, smaller units
def process_order(order):
    validate(order)
    apply_discount(order)
    persist_and_notify(order)

def persist_and_notify(order):
    save_to_db(order)
    send_confirmation(order)
    log_metrics(order)
    update_loyalty_points(order)

Incremental refactoring

This means making small, manageable changes over time rather than attempting a massive overhaul in one shot. The system should remain functional at each step (even internally in transition). The advantage is risk mitigation: each small change is less likely to go wrong, and it’s easier to pinpoint and fix if it does.

Incremental delivery lets you confirm changes in production and makes diagnosing issues easier since you’re only changing one small thing at a time​. It also means the system keeps running during the refactor, so there’s less pressure to rush to “get the system back to working condition”​. If priorities shift, you can pause after some increments and still have a working product.

Big bang refactoring (Rewrite)

This is the “tear it down and rebuild” approach. You stop adding new features, possibly freeze the code for a period, and devote a considerable effort to redesigning or rewriting a significant portion (or the entirety) of the system. The idea is to emerge on the other side with a brand new, clean system.

So when (if ever) is a big bang justified? Perhaps when the existing system is truly untenable – for example, an outdated technology that must be replaced (such as a platform that can’t meet new performance or security requirements or code written in a language no longer supported). Even then, wise teams often simulate a big bang by breaking it into stages or developing the new system in parallel.

Whenever possible, favor an incremental refactoring strategy. Teams successfully pull off massive transformations by treating the big refactor as a series of mini-refactors under a shared vision.

3. Breaking Down Monolithic Code

Many complex codebases start life as a single monolithic application, one deployable, a single code project, or a tightly coupled set of modules all maintained and released together.

Over time, monoliths can become unwieldy, builds take forever, a change in one area can unintentionally affect another, and teams can be complex to scale because everyone is stepping on each other’s toes in the same code. A common refactoring challenge for senior engineers is modularising or splitting a monolith into more manageable pieces.

# define the interface
class PaymentProcessor:
    def charge(self, amount): ...

# old implementation
class LegacyProcessor(PaymentProcessor):
    def charge(self, amount):
        # original code

# new implementation behind a feature flag
class NewProcessor(PaymentProcessor):
    def charge(self, amount):
        # cleaner code

def get_processor():
    if config.feature_new_payment:
        return NewProcessor()
    return LegacyProcessor()

# usage remains the same
processor = get_processor()
processor.charge(100)

Strategies for modularization.

• Layer separation: Start by enforcing logical layer boundaries. For example, separate the user interface code from business logic and separate business logic from data access. In a messy monolith, these concerns often get mixed together. By organizing the code into layers (even within the same repository), you can limit the ripple effect of changes.

• Domain-based modularization: If your system spans multiple business domains or functional areas, consider splitting along those lines. For example, an e-commerce monolith might be separated into modules like Accounts, Orders, Products, Shipping, and so on.
  Each could become a subsystem or a package. The goal is to minimize the information these modules need to know about each other’s internals (high cohesion within modules and clear APIs between them).

• Microservices or services extraction: In recent years, the trend has been to break monoliths into microservices, independent services that communicate over APIs. This form of architectural refactoring can significantly improve independent deployability and scalability. But it’s a significant undertaking with complexities (distributed systems, network calls, and so on). If you decide to go this route, do it gradually.
  A proven method is the strangler fig pattern mentioned earlier: you pick one piece of functionality and rewrite or extract it as a separate service, redirect traffic or calls to the new service. At the same time, the rest of the monolith remains intact and iteratively does this for other pieces​.

• Modular monolith: Not every system needs to go full microservices. There’s an approach called a modular monolith, essentially structuring your single application into well-defined modules that communicate via explicit interfaces (almost like internal microservices but without the overhead of separate deployments).

This can give you many microservices' advantages (clear boundaries, separate development responsibility) while avoiding operational complexity.

• Identify shared utilities vs. truly independent components: In breaking down a monolith, some code is widely shared (like utility functions or cross-cutting concerns such as authentication). It might make sense to factor those into libraries or services first, as they will be needed by whatever other pieces you split out.

While breaking down a monolith, maintaining functionality during the transition is essential. Techniques like backward compatibility (discussed next) and thorough testing will be your safety net.

Finally, be prepared for the team workflow to change. If you move to microservices, teams might take ownership of different services, requiring more DevOps and communication across teams. If you keep a modular monolith, enforce code ownership or review rules to keep the modules from tangling up again (for example, you might restrict direct database access from one module to another’s tables, and so on).

4. Ensuring Backward Compatibility

A critical concern during large refactoring is: Will our changes break existing contracts?

In other words, can other systems, modules, or clients that rely on our code work as expected after we refactor? Backward compatibility is especially important if your codebase provides public APIs (to external customers or other teams), data persisted in a certain format, configuration files that users have written, etc.

Here are some strategies and considerations to maintain backward compatibility:

Suppose you have a widely-used function like send_email(to, subject, body). You want to refactor the internal logic to support additional features like HTML formatting, but you don’t want to break existing callers.

Instead of changing the function signature, you keep the public API unchanged and delegate to a new internal function:

# original API
def send_email(to, subject, body):
    # send mail...

# refactored internals, keep signature
def send_email(to, subject, body):
    sendv2(to=to, subject=subject, body=body)

def sendv2(to, subject, body, html=True):
    # new implementation with HTML support

The internal send_email_v2() function adds new capabilities like HTML formatting, but older code using send_email() still works without any modifications.

If you're introducing a new, improved version like send_email_v2(to, subject, body, html=True), it's good practice to:

• Mark the old version (send_email) as deprecated in documentation.

• Ensure the old version internally calls the new one.

• Give other teams time to migrate at their own pace.

Use versioning for external APIs

If your system provides an HTTP API or similar to external clients, the safest route for major changes is to version the API. Introduce a v2 API endpoint for the refactored logic, keep v1 running (maybe internally calling v2 or using a translation layer). Clients can move to v2 at their own pace.

It’s extra work to maintain two APIs temporarily, but it prevents a breaking change from angering users or causing outages. Always communicate changes clearly and provide migration guides if applicable.

Have a clear deprecation policy

Make sure there’s a policy (and communication) around how long deprecated features will be supported. For internal APIs, maybe it’s one release cycle. For external ones, maybe multiple cycles or never removal without a major version bump. A good practice is to announce deprecation early.

If you’re exposing an HTTP API, consider introducing a new versioned endpoint (for example, /api/v2/send_email) and maintain the older /api/v1/send_email temporarily. Internally, v1 might call v2 with default parameters, ensuring behavior stays consistent for existing clients.

In summary, maintain backward compatibility whenever possible, and implement a clear deprecation policy for anything you do change​.

Write adapter or compatibility layers

In some cases, you can write an adapter to bridge old and new systems. For instance, suppose you refactor the underlying data model of your application, but you still have old configuration files in the old format. Rather than forcing all those files to be rewritten immediately, you could write a small adapter that translates the old format to the new one at runtime (or during startup). This way, old data continues to work.

Test for compatibility

Include tests that specifically ensure backward compatibility. For instance, if you have a public API, keep a suite of tests using the old API contracts and run them against the refactored code, they should still pass.

In summary, ensure that as you refactor, the external behavior and contracts remain consistent. This careful approach protects your users and downstream systems, allowing you to reap the internal benefits of refactoring without causing external chaos.

5. Handling dependencies and tight coupling

One of the hairiest aspects of refactoring a large codebase is dealing with deeply interdependent code. Complex systems often suffer from tight coupling. Module A assumes details about Module B and vice versa, global variables or singletons are used all over, or a change in one place ripples through half the codebase.

Reducing coupling is a significant aim of refactoring because it makes the code more modular, meaning each piece can be understood, tested, and changed independently. So, how do we gradually loosen the coupling in a legacy system?

Let’s go over some strategies to reduce coupling.

Introduce interfaces or abstraction layers

A very effective way to decouple is to put an interface between components. For example, if you have a class that directly queries a database, introduce an interface and have the class use that instead. The underlying database code implements the interface.

# before: direct instantiation
class OrderService:
    def __init__(self):
        self.repo = OrderRepository()

# after: inject dependency
class OrderService:
    def __init__(self, repo):
        self.repo = repo

# wiring up in application startup
repo = OrderRepository(db_conn)
service = OrderService(repo)

Now, that class no longer depends on how the data is fetched. Applying the dependency inversion principle depends on abstractions, not concretions.

Use dependency injection

Once you have interfaces, use dependency injection to supply concrete implementations. Many frameworks support DI containers, or you can do it manually (passing in dependencies via constructors). Dependency injection means code A doesn’t instantiate code B itself – instead, B is passed into A.

This approach also makes unit testing easier (you can inject mock dependencies).

Facades or wrapper services

If a particular subsystem is heavily entangled with others, consider creating a Facade, an object that provides a simplified interface to a larger body of code. Other parts of the system are then called the Facade, not the many internal methods of the subsystem. Internally, the subsystem can be refactored (even split into smaller pieces) as long as the Facade’s outward interface remains consistent.

This is similar to how microservices work (other services don’t care how one service is implemented internally – they just call its API), but you can do it in-process, too.

Gradual replacement (Parallel Run)

If a specific component is to be replaced with a new implementation, it can help to run them in parallel for a while. For instance, if you have a spaghetti module that you want to redo correctly, you could leave the spaghetti code in place for legacy calls but start routing new calls to the new module.

The result is a codebase where changes in one area (hopefully) won’t unpredictably break another, a key property of a maintainable system.

6. Testing Strategies (Safely Refactoring with Confidence)

A robust testing strategy will give you the confidence to make sweeping changes because you’ll know quickly if something important breaks. Here’s how to approach testing in the context of a large refactoring:

Establish a baseline with regression tests

Before you even begin refactoring a particular component, make sure you have tests that cover its current behavior. You're lucky if the codebase already has a good test suite, but many legacy systems have inadequate tests.

One of the first tasks in those cases is often writing characterization tests. A characterization test is a test that documents what the system currently does, not what we think it should do​.

As Feathers says, “a characterization test is a test that characterizes the actual behavior of a piece of code.” This allows you to take a snapshot of what it does and ensure that it doesn’t change​.

This gives you a safety net so you can refactor with confidence that you’re not introducing regressions​. Use automated test suites to help things run smoothly (unit, integration, end-to-end).

Continuous integration (CI)

It is highly recommended that testing be integrated into a CI pipeline that runs on every commit or merge. This way, you catch a bug during refactoring as soon as you introduce it, tightening the feedback loop.

Canary releases and feature flags

Beyond pre-release testing, consider strategies for safely deploying refactored code. A canary release involves rolling out the change to a small subset of users or servers first, observing it, and then gradually expanding​.

This is great for catching issues that tests might miss (for example, performance issues or edge cases in production data). If the canary looks good (no errors, metrics are healthy), you proceed to full rollout. If not, you rollback quickly—with only a small impact scope.

Performance and load testing

If performance is a concern, incorporate performance tests into your strategy. This can be done in a staging environment. You might reconsider your approach or optimize the new code if you see a significant regression.

Testing legacy code lacking tests

If you’re dealing with a part of the system with zero tests (not uncommon in older code), prioritize getting at least some coverage there. There are also techniques like approval testing (where you generate output and have a human approve it as correct, then use that as a baseline for future tests). The key is not to refactor entirely in the dark; give yourself at least a flashlight in the form of tests!

In sum, a strong testing strategy is non-negotiable for refactoring complex systems. It’s your safety net, early warning system, and guide to know that your “cleanup” hasn’t broken anything vital.

7. Refactoring Without Breaking Performance

A common concern when refactoring is whether these cleaner code changes will make my system slower or more resource-hungry. Ideally, refactoring is about the internal structure and shouldn’t change external behavior, and performance is part of the behavior.

In theory, performance should remain the same if you don’t change algorithms or data structures in a way that affects complexity.

In practice, though, performance can be inadvertently affected by refactoring. The new code may be more readable but uses more memory, or perhaps a critical caching mechanism was removed in the spirit of simplicity.

Senior engineers need to be mindful of performance-sensitive parts of the system when refactoring and take steps to avoid regressions (or even improve performance where possible).

Here’s how to refactor with performance in mind:

Identify performance-critical code paths

Not all codes are equal regarding performance impact. If you refactor them, treat it almost like a functional change: you must re-measure performance afterwards. You have more leeway for parts of the code that run rarely or are not bottlenecks.

Use profiling before and after

A profiler is a tool that measures where time is spent in your code or how memory is allocated. It’s beneficial to run a profiler on the code before refactoring a module to see how it behaves, and then run it after to compare. If you see, for example, that after refactoring, a function now shows up as taking 30% of execution time (when it was negligible before), that’s a red flag. Maybe the new code calls it more times than before.

import cProfile, pstats
from mymodule import slow_function

def profile(fn):
    profiler = cProfile.Profile()
    profiler.enable()
    fn()
    profiler.disable()
    stats = pstats.Stats(profiler).strip_dirs().sort_stats('cumtime')
    stats.print_stats(10)

# run before refactor
profile(lambda: slow_function())

# after you refactor slow_function(), re-run and compare stats

When possible, improve performance through refactoring

On the flip side, refactoring can help performance.

For example, by refactoring duplicated code into one place, you can use better caching in that one place. So, watch for performance improvement opportunities that arise naturally as you refactor.

Performance should be treated as part of the “external behavior” that needs to be preserved in a good mindset. Refactoring should ideally not make things slower for users. To ensure that, incorporate performance checks into your plan, especially for critical sections. Measure, don’t guess. The end goal is a codebase that is both clean and fast enough.

8. Automate Code Reviews with AI tools

Refactoring code is an ongoing process, not a one-time event – AI code review tools help enforce clean-code standards, catch smells early, and reduce the repetitive tasks that can bog down human reviewers. This frees your engineers to focus on deeper architectural or domain-specific issues.

One powerful option is CodeRabbit, an AI-driven review platform designed to cut review time and bugs in half.

Here’s how it works and why it can boost your refactoring workflow:

AI-powered contextual feedback

CodeRabbit analyzes pull requests line by line, applying both advanced language models and static analysis under the hood. It flags potential bugs, best-practice deviations, and style issues before a human opens the PR.

Some other features include:

• Auto-generated summaries and 1-click fixes – Summarize large PRs and apply straightforward fixes instantly.

• Real-time collaboration and AI chat – Chat with the AI for clarifications, alternate code snippets, and instant feedback.

• Integrates with popular dev platforms – Supports GitHub, GitLab, and Azure DevOps for seamless PR scanning.

CodeRabbit even has a free AI code reviews in VS Code and with this VS Code extension, you can get the most advanced AI code reviews directly in your code editor, saving review time, catching more bugs, and helping you in refactoring.

Summary

Refactoring a complex enterprise codebase is like renovating a large building while people still live in it without collapsing the structure.

Refactoring should be an ongoing process. You prevent the codebase from decaying by incorporating these practices into your regular development (perhaps allocating some time each sprint for refactoring or doing it opportunistically when touching your code). Each minor refactoring should not be too complex, and the cumulative effect is significant.

As Martin Fowler puts it, a series of small changes can lead to a significant improvement in design.

That's it for this blog. I hope you learned something new today.

If you want to read more interesting articles about developer tools, React, Next.js, AI and more, then I'll encourage you to checkout my blog.

Some of the new and interesting articles I've written in the last 24 months.

• Cursor vs Windsurf

• How to Implement Role-Based Access Control in Next.js

• AI Code Reviewers vs Human Code Reviewers

• How to Build a Custom Video Conferencing App with Stream and Next.js

• How to Perform Code Reviews in Tech – The Painless Way

You can get in touch if you have any questions or corrections. I’m expecting them.

And if you found this blog useful, please share it with your friends and colleagues who might benefit from it as well. Your support enables me to continue producing useful content for the tech community.

Now it’s time to take the next step by subscribing to my newsletter and following me on Twitter.

This guide provides a straightforward and flexible template for code reviews that you can apply to your engineering team. The only requirement is that your app code is open source.

You can test a TypeScript workflow, Java workflow, Python workflow, PHP, Ruby or even some wacky web stack you invented. And it doesn’t matter if you’re developing on Windows, Linux, or Mac. Best of all, you don’t have to perform convoluted configuration or install software beyond a yaml.

I’ve been in engineering for the last 15 years, and code reviews have a bad reputation. We’ve all witnessed or lived through horror stories where sometimes it feels like every previous line gets torn to shreds.

So, what can you do differently? How can you make reviewing your code painless so that even the biggest nitpick on your team has nothing but praise?

After participating in code reviews for a decade, taking code reviews less personally is the single biggest thing you can do to improve your code. Why? Because all software is iterative. Even “perfect” code will eventually become outdated. Instead of thinking of it like a graded assignment, think of it as a part of the process.

Table of Contents:

• Prerequisites

• What is a Code Review?

• What is the Purpose of a Code Review?

• Why is Doing Code Reviews Hard?

• Can AI Replace Code Reviews?

• What to Focus on During a Code Review

• Code Review Best Practices And Process

• What is CodeRabbit?

  • How Does CodeRabbit Help?

  • A GitHub Repo to Test

  • Additional Examples

• Conclusion

Prerequisites

This tutorial uses free, open-source tools. You’ll need to have a GitHub account to help you make your code reviews more pleasant and valuable.

What is a Code Review?

The term “code review” can refer to various activities, from simply reading code over your teammate’s shoulder to a 10-person meeting where you dissect code line by line. I use the term to refer to a formal and written process, but not so heavyweight as a series of in-person code inspection meetings.

In a project where you work on a repository with other developers, after you complete your work, you commit, push, and create a pull request on the VCS, most likely using Git commands. Then, everyone reviews the pull request to determine whether it’s okay to use. If so, they approve it, and that code gets used in the project.

What is the Purpose of a Code Review?

Code Reviews are a tool for knowledge transfer. They help make devs more efficient when doing maintenance on a part of the system they didn't write.

When you review a pull request, it’s an opportunity to iron out issues before they become technical debt.

Code reviews can also be a good setting for mentoring junior developers.

Now, let’s discuss what is not the purpose of a code review:

• Finding bugs. That's what tests (unit, integration, e2e, api, and so on…)are for.

Nitpicking on style issues – settle for one style and use formatters or AI tools to enforce it. Just keep in mind that there are many things that an AI tool cannot check. Code reviews are an excellent place to ensure the code is sufficiently documented or self-documenting.

Do you want to know how you can check this? Return to the code you wrote 6-12 months ago and try to understand what it was written to do.

If you understand it quickly, that means it's readable, and the code review was done properly and in a helpful manner.

Why is Doing Code Reviews Hard?

Despite their importance, many devs don’t like doing code reviews – in part because they can be challenging, especially if you’re not following best practices.

Here are some pain points I’ve observed during my years of participating in code reviews:

• When people talk about code reviews, they focus on the reviewer. But the developer who writes the code is just as crucial to the review as the person who reads it.

• Doing a code review is not an automatic routine for a developer.

• The reviewer may sometimes just do a partial review and add new comments at every pass, even on code in the previous review(s) that remained untouched.

• Sometimes, the code reviewer may not clearly express their expectations.

• Multiple code reviewers can often have diverging opinions, leading to (too) long discussions.

• The developer does not understand the comments from reviewers and requires back-and-forth discussions.

• The developer addresses code review comments differently than agreed upon during the review process.

These pain points often bottleneck our development velocity. But recent advances in AI-assisted code review tools have started addressing these common friction points in our PR workflows.

Let's explore how AI-powered tools, along with some best practices, can address these review challenges and optimize your development workflow.

Can AI Replace Code Reviews?

While AI hasn’t replaced human code reviews, it is a powerful force multiplier in the review process.

Here's how: AI code reviews excel as a preliminary screening tool, catching common issues before human reviewers see the code. This becomes especially valuable in open-source projects where maintainer bandwidth is limited.

I recently started using AI code reviews on a case-by-case basis for my projects.

AI tools improve my existing workflows, reduce failure rates by detecting logic errors early on, and boost productivity. So I’ve added it to my CI/CD pipelines. It doesn't have to be perfect at detecting logic errors, as long as its false positive rate is very low (ideally as close to 0 as possible).

Most importantly, AI reviews respect the golden rule of 'value your reviewer's time' by handling routine checks. This allows human reviewers to focus on architecture, business logic, and complex edge cases.

This approach positions AI as a complementary tool that augments rather than replaces human expertise in the code review process.

What to Focus on During a Code Review

When reviewing code, try to prioritise what matters most using the Code Review Pyramid. This is a framework that helps you focus your attention where it creates the most value.

Think of it like building a house — start with the foundation before worrying about paint colours.

The pyramid has five layers, from most critical (bottom) to least critical (top):

1. API Semantics: Core design decisions that affect users

2. Implementation Semantics: The code's functionality, security, and performance

3. Documentation: Clear explanation of how to use the code

4. Tests: Verification that everything works as intended

5. Code Style: Formatting and naming conventions

Source: The Code Review Pyramid by Gunnar Morling

Remember: if you want to catch issues/bugs, there are more appropriate processes for that. That is why we have automated testing, canary releases, testing environments, and so on.

In my personal opinion, using code reviews as a bug catching tool is somewhat of an anti-pattern where you're compensating for a development process that may be lacking some key steps/processes.

To me, a code review is much more about managing technical debt and ensuring that quality is produced, while shipping more features.

In doing a code review, you should make sure that:

• The code is readable

• It has appropriate unit tests

• The developer used clear names for everything

• The code is well-designed and isn’t more complex than it needs to be

• Test cases make sense and have comprehensive coverage

• It’s something the team can maintain in the long run

• There are no architectural issues that will block the team

• The code fits the team's idea of quality

• You’re thinking about what you can learn from the PR

• You’re sharing any knowledge the developer might use in their PR

• You’re thinking about how you can empower the dev through your positive feedback

• The PR has a clear changelist description

Code Review Best Practices And Process

There is no general rule in engineering for code reviews, as what you’ll need to focus on depends on many factors. You can and should set up the process according to your company standards and way of working as a team.

Here are some factors you’ll need to think about before setting up a code review process:

• The size and type of company you’re in (for example a startup vs a large corporation)

• The number of developers on your team

• Your budget

• The timeframe you’re working with

• Your and your team’s workloads

• The complexity of the code

• The abilities and skills of the reviewer(s)

• The availability of the reviewer(s)

As an example, at my work we have a very simple rule: all code changes must be reviewed by at least one developer before a merge or a commit to the trunk.

Code reviews need a systematic approach, but maintaining consistency across every PR is challenging. It’s useful to let computers handle repetitive checks (style, formatting) while humans focus on what matters most: architecture and logic. This balanced approach makes reviews both thorough and sustainable.

Take a look at this example. It shows how we can optimize our code review process by intelligently delegating tasks between humans and automated tools. The diagram below illustrates a typical code style review workflow, comparing manual human review steps against automated tooling.

The diagram shows a real problem we all face in code reviews. See the left side? That's we humans doing manual formatting checks: finding weird spaces, fixing indents, writing comments about it... pretty tedious stuff. But check out the right side: that's where tools like Prettier just fix these formatting issues automatically.

No meetings, no back-and-forth – just done. That's why I started using CodeRabbit, which is a dev tool that caught my attention recently.

What is CodeRabbit?

The CodeRabbit docs describe the tool pretty effectively, so I’ll just leave this here:

CodeRabbit is an AI-powered code reviewer that delivers context-aware feedback on pull requests within minutes, reducing the time and effort needed for manual code reviews. It provides a fresh perspective and catches issues that are often missed, enhancing the overall review quality. – from the CodeRabbit docs

How Does CodeRabbit Help?

Let me walk you through a real example. When you submit a PR, CodeRabbit:

1. Performs a PR summary on the fly:

• First, it gives you a quick overview of what changed.

• It also explains the impact in plain English (great for non-tech folks in your team).

• Then it includes a detailed walkthrough of file changes.

2. Does a “Smart Code Review”:

• It drops comments right on the specific lines that need attention.

• It also suggests fixes in diff format that you can apply them with one click.

• And it shows what commits and files it checked (which is helpful for tracking review coverage).

3. Give you interactive feedback:

• You can chat with it right in the PR comments.

• You can ask it questions about specific code changes to get more details.

• And it remembers your team's patterns and preferences which is super helpful for consistency’s sake (which I discussed above).

4. Extra Helpful Features:

• CodeRabbit validates changes against linked GitHub/GitLab issues.

• It creates sequence diagrams to visualize changes.

• And it can perform one-click fixes on applications for simple issues.

I first discovered CodeRabbit last month while I was searching for something else on GitHub. I accidentally came across it and I was surprised by how many people are already using it.

I instantly signed up because I was looking for exactly such a solution which could help me and my team out with our reviews.

I read through the CodeRabbit docs and was very impressed.

Getting started using it is pretty much a plug and play process.

In the next section, we’ll go through the quick steps you can follow to enable CodeRabbit using an example repo.

• Sign up at coderabbit.ai using your GitHub account.

• Go to Add Your Repository.

• And that's it. CodeRabbit starts reviewing your PRs automatically.

A GitHub Repo to Test

As an example GitHub repo to test, we’ll use devtoolsacademy: my blog on everything about awesome developer tools.

First, visit the CodeRabbit login page and login via GitHub.

Next, add CodeRabbit to some of your public GitHub repositories.

Now, CodeRabbit is fully integrated and ready to do code reviews on your selected repo.

Yes: it’s that simple and fast. And in my opinion, it’s one of the main reasons the tool is so useful.

Here are some sample PRs for you to check out:

• https://github.com/tyaga001/devtoolsacademy/pull/10

• https://github.com/tyaga001/devtoolsacademy/pull/13

• sartography/spiff-arena#1233 (comment)

• kmesiab/equilibria#1 (comment)

• kamiazya/web-csv-toolbox#60 (comment)

• openreplay/openreplay#1858 (comment)

• ls1intum/Artemis#8037 (comment)

Additional Examples

Conclusion

Everyone’s code needs reviewing. Just because someone is the most senior person on the team does not mean that their code doesn’t need to be reviewed.

In this article, I talked about code reviews along with some common pain points. I then showed you how you can leverage CodeRabbit to iterate quickly through your code reviews and focus more on business.

Further reading

In this article I talked about basic intro to CodeRabbit, because that was my use case with my blog.

For more advanced functionality, check out the official CodeRabbit docs or read their blog.

Before I End

I hope you found it helpful learning how to use AI tools for code reviews.

If you like my writing, these are some of my other most recent articles.

• Neon Postgres vs Supabase

• MongoDB vs. PostgreSQL

• Supabase vs Clerk

• How I Built a Video Conferencing App with Stream and Next.js

• How to Implement Fine-Grained Authorization in Java

Follow me on Twitter to stay updated on my open source projects.

No worries, I’ve been there myself when I started getting more comfortable in the open source world. So, grab a chair, your favorite snack or beverage (I highly recommend water. It’s fresh and good for you! 😉), and your notebook (or in this case, your laptop). Because I’m about to share five techniques that will have you reviewing pull requests like a boss.

Neil deGrasse Tyson says "Let's Do This!" to give you confidence!

Feedback Technique 1: The “Show and Tell” Review

For this technique, you provide screenshots or links to other sources that help explain the benefit of the new approach that you’re suggesting to your fellow contributor. Here's how it's done:

First, take a screenshot of the thing you want to convey in your feedback. I highly recommend using Fireshot. It's an easy way to take screenshots on your computer.

This is a GIF of someone taking a screenshot of an image via a Mac

Then, go to the repository of your choosing and click on Pull Requests

A yellow circle surrounds the third tab called "Pull Requests". Click there to pick the PR you want to review.

Once you pick the pull request you want to review, click on Files Changed:

Add your comments, then drag and drop your image in the textbox.

In a textbox,

Click Submit review and volià you're done! 😊

There's a green button saying "Submit Review". Click on it after you finish your review.

In my experience, this type of review is helpful when giving feedback on pull requests that require adding a feature to an open source project's website (for example a logo) or an image on one of its Markdown files. It can help the person see how their contribution impacts the overall project.

It's like if you were watching a commercial for a facial soap and you see the scenes where they show close ups of how the product makes your skin healthier. Showing someone an image of what needs fixing can help convey what's going on more clearly.

Speaking of skin, here's another strategy where your shifting powers, I mean adaptability skills, can be helpful.

Feedback Technique 2: The “Chameleon” Review

The “Chameleon” is a feedback giving technique where you adapt your PR review based on the type of contribution that your peer is making. It's like how a chameleon changes their skin color to fit in with their surroundings (minus the hiding from predators part of course 😉).

For example, if you’re reviewing a pull request that's text-based like the one in the image below, I highly recommend giving feedback via dialogic questions (for example, how does this resource stand out from other courses that teach JavaScript?).

This technique is helpful because it encourages the recipient of your review to think deeply about their contribution. It also teaches you how to tailor your feedback based on the type of pull request that you are reviewing.

Now that you know the Chameleon, the next technique you'll learn is one that can help you swipe through fields, I mean, long lines of code.

Feedback Technique 3: Two Peas in a Pod

The “Two Peas in a pod” is a PR review technique where you comment on one line of code in the conversation while another contributor gives feedback on another line code in the same pull request.

Here's an example:

Two contributors comment on one line of code

My comment on a different line of code on the same pull request

As shown in the first picture, it's helpful to point out why certain methods won't work and discuss some alternatives the pull requester can use. Additionally, consider commenting on a different way to improve the PR once you picked the line of code you want to improve.

When using this method, I highly recommend encouraging you and your fellow reviewer to pick a line of code that suits your strengths because it'll make giving feedback easier.

Since my background is in writing and education, I decided to comment on the text-based line that you see in the second picture whereas the other contributors focused on improving the image element due to being more experienced in coding.

This method helps you develop your written communication skills and ultimately makes reviewing pull requests less stressful. That's two benefits for the price of one! 😊 Cool right? 😉

Speaking of lessening your stress, I have another strategy that will have you reviewing pull requests like the Flash from The Justice League!

Feedback Technique 4: Teach’em Review

This “Teach’em” review is a PR review technique where you pretty much instruct your peer on how to improve their PR instead of just pointing out the issues in the PR.

Here are some examples:

My suggestion: turning the <div> element into a more semantic HTML element.

I give tips to the contributor how to make their code-based contributions more accessible

When using this technique, I highly recommend pointing out an area of improvement. Then, you can briefly teach a strategy they could use in the future.

This approach can help improve your coding skills and develop your written communication skills, which is very useful in the world of tech.

Now, there’s just one more technique that will help you upgrade your PR reviewing skills.

Feedback Technique 5: The “Suggestion” Review

If you're focusing on other open source projects, have a deadline for an assignment, or you're just tired from a long day at work but want to review pull requests, this feedback giving technique will be the ultimate tool in your open sourcer kit. It focuses on giving constructive feedback through your review.

Here's how it's done:

1. Click on the Files Changed tab of a person’s pull request:

There are four tabs underneath a person's pull request. The final one "Files changed" is circled in yellow. That's the tab you pick in the first step

2. Hover over the line of code you want to review and click on the blue plus sign:

There is a blue plus sign over the line code that has a yellow covering. This indicates that you have picked a line of code you want to review.

3. Click on the file icon that has a plus on top and a minus sign on the bottom:

4. Rewrite the line of code, improving it as you think is necessary:

There's a yellow arrow point to an area that says ```suggestion. This where you rewrite the line of code that you choose to review.

5. Click on Add Single Comment

There is a yellow arrow pointing at the "Add single comment" button. This is what you click on to insert your suggestion into the person's line of code.

6. Write your comment in the textbox, click on Request Changes, and Submit Review.

The first arrow points to a textbox, the place where type your comment. The second arrow points the last radio button labeled "Request Changes". The final arrow points at a green button labeled "Submit review".

I highly recommend using this feedback technique for pull requests that are text-based because it will show your suggestion to the person who made the pull request, which is especially helpful if you are too tired or don't have time to give a brief grammar lesson (trust me, I've been there).

Like the other feedback techniques I previously mentioned, the suggestion review can also help you improve your detail-oriented skills as it encourages you to think of the best way to convey your feedback. It's pretty awesome! 😊

Conclusion

Congratulations, you now have five feedback giving techniques in your Open Sourcer toolkit!

Before I let you go, I want you to remember this. Open source contributions begin and end with you, so use your powers wisely. Now, go out there and be the best pull request reviewer in open source!

Deadpool and his crew is walking slowly towards their next challenge. Be like Deadpool with your pull request reviews! :)

Credits

Let's Do This GIF by National Geographic

Super Hero Walking GIF by 20th Century Fox Home Entertainment

Screenshot GIF from "How to Take a Screenshot on a MacBook" by Hung Nguyen

But does this mean we're doing things right? Familiarity often leads to sloppiness and overlooking the basics.

In this article, we will explore

• How to write meaningful Git commit messages
• How to create efficient pull requests (PR)
• How to get really good at the code review process and some best practices to follow

Pre-Requisites:

I prefer to use VS Code as my code editor. I use this for my Git editor too. I find it easier to write commit messages within the same place as I code. It also gives me more space to write commit messages and descriptions.

If you haven't already done this, follow these steps to make VS Code your default git editor.

1. Open VS Code and in the command palette search for

Shell Command: Install 'code' command to PATH

2. Then run the following command in your terminal:

git config --global core.editor "code --wait"

Now when you run git commit or git -config --global -e it will open the Git editor within a file in VS Code.

Note: All given commands are to be run within the terminal (whether that be your terminal of choice, or the integrated terminal within VS Code).

How to Write Meaningful Commit Messages

When committing your code, it's helpful to write useful commit messages. Here are some tips and best practices to help you do so.

Use Imperative Commands

Prefix your commit messages with imperative commands such as: fix, refactor, add, and remove

This is because you should be able to suffix a commit message to the phrase

"If applied, this code will..."

and inform other developers what it will do, for example:‌

If applied, this code will fix issue with login button not showing

Keep It Brief

You're not writing a monologue, so keep it brief. As a general rule, a commit message should not exceed 50 characters.

Put yourself in the developer's or reviewer's shoes. Try and think about what you would want to know if you were looking at the Git log on this repo.

• What work did you complete?
• Why did you do it?
• What effect does it have on the code base?

Here's an example of a concise yet informative commit message:

fix issue with login button not showing

How to Keep Commit Messages Short but Helpful

Within your commits, you can include a commit description, allowing us to add even more detail / context as to what you did.

Add an empty line underneath the commit message, and begin writing a description on line 3. It looks like this:

fix issue with login buttton not showing

- update login form validation
- update login styling for showing the button

Now when other devs are reviewing Git logs, commits, or needing to revert code they have a better indication of what effect will take place, and whether it will cause any breaking changes.

Examples of Unhelpful Commit Messages

On the other hand, here are some examples of ineffective commit messages:

• fixed bug – There is no reference to what bug has been fixed exactly, so it adds no value to the git history / logs. It will make reviewing previous commits extremely difficult, and painstaking. As a developer you would have to open each commit like this to understand what it's actually doing.
• refactored due to PR comments – This message gives us no information about what was changed. We would have to hunt down the pull request to gather any context around the changes made, or again open up the commit.
• fixing previous commit – Lack of context again
• made tests pass – Which test file was updated? You should at least give the name or area of tests that have been fixed.

All of these are poor examples of commit messages due to their ambiguity, lack of information, and lack of context. They force team members to do more work in order to understand what's going on, which is not acceptable in any team.

How to Develop a Commit Strategy

You may think committing code is as straightforward as just committing and pushing the code. But there's a little more to it than that.

Let's talk about how you can develop a useful commit strategy to stay consistent and make useful commits.

Make small, specific commits

Smaller commits make it easier to revert code to a previous state if there's a problem. If your commit affects too many areas, reverting back could mean losing a lot of code.

It's also far easier for reviewers to understand what the code is doing if it's related to one purpose.

Let's see a real life example of how this would work. Firstly we need to add our related file changes. We can view which files have been changed within our branch by running git status

Example of output from git status

As you can see there are various files that have been changed / added to the project. However, they're all for different areas of the project. Following the golden rule of keeping commits small and relatable, let's take a look at how we can put that into practice.

Using the git add followed by the file names we can commit only the files which are related. We do this by using the git add command , plus the names of the files we wish to add one after another like so:

git add login.test.ts login.ts

If we check git status now, we'll see the two staged files:

Example of Git status output after staging files

We have the files, now to create our commit. As always, we'll utilise git commit which will open the git editor within VS Code (as we set it up like this earlier). If you skipped this step, the commit will open in your preferred editor.

We'll add a commit message of the changes:

Fix issue with login buttton not showing

There you have it, a small related file commit. The commit message tells us exactly what we've done, where the issue resided, along with some small context of what the issue was.

Bad Example of a Commit

Since we've done it successfully, lets take a look at a bad example:

So imagine we've done all this work, and the developer has staged all the files at once, using the git add -A command which stages all changed/added files.

Example of multiple unrelated files staged for commit

They now create a commit message utilising the one liner Git command:

git commit -m 'Updated various areas such as validation, registration and products pages'

Why is this so bad?

1. Firstly there's too much happening within this commit. Too many files mean that if we need to revert the validation changes, only we can't. We'd have to revert the whole commit losing the products and registration changes.
2. The commit message is lengthy. We can remove unnecessary words like 'various areas such as'. It adds no value to the commit message, and takes up characters that could be used for more context.
3. We're not using the imperative voice as mentioned earlier. We should change "Updated" to 'Update'.

If applied, this code will fix the submit button not showing on the login

Midway Recap

So at this point, we've learnt:

• How to formulate a useful commit message
• How to formulate an effective commit strategy to allow us to easily keep track of related changes, and a more maintainable code base.
• What makes a bad commit

How to Create Efficient Pull Requests

Decide when to push

Pushing is the act of getting your commits up to the server / remote origin ready to create a Pull Request (PR). I recommend pushing as soon as the current feature or bug is done.

Also, it's a good idea to keep your PR's small, containing only related commits. As an example if you had the following commits:

• Add new product search component
• Add unit tests for product search component
• Add documentation for product search component

Because all these commits are related to the same component, it's recommended to pull them together into one PR.

Whereas, something like:

• Fix bug within login screen
• Refactor registration page for performance
• Update validation tests for login form
• Update login tests for forgotten password
• Update unit tests for product search component

Should not go in the same PR, as there's far too much going on. These commits should have been grouped into several PR's containing relevant commits at the time.

If this was the Git log for a branch, you would be unable to create a pull request with solely relatable files, due to the order in which the commits have been created, as you can't simply inform Git that you want commits 1,3 and 4 to go into this PR.

Keep it small

Remember – like your commits, keep your PR's small. Nobody wants to wade their way through a 50+ file pull request. All that'll happen then is that your review will suffer from the common "Looks good to me".

When you create a large PR, it translates to "I couldn't be bothered to look through all these files, but skimming through them it looks ok". With a smaller PR, on the other hand, it means exactly what it says.

Sometimes large pull requests are unavoidable, like when updating base functions and such. However, you should try to be aware of how you can limit the impact on other developers.

How to Get Your Branch Ready For Code Review

The exact process of creating a Pull Request will differ depending on which version control hosting platform you are using, but the concepts are the same.

First, you should check out the main branch from your repo. Then run a git pull, which will get all the latest code from main onto your local dev system.

Once you've done that, you can go back to your own branch using git checkout and the name of your branch , for example git checkout login-fixes.

Now we need to get the main branch code into ours. We can do this utilising the git merge command.

git merge main

If there were changes, that is there were files in the pull from main, you will need to create a "merge commit". That's just another commit to your own branch containing the merged changes.

Simply create another commit, with a message explaining you've merged from main like so:

git commit -m 'Merge main into login-fixes'

Push your changes back to the remote server using git

How to Create the Pull Request

The easiest way to do this is via the web interface on your preferred version control platform. In this example we'll be utilising GitHub.

Simply navigate to your repo, and then click the Pull Requests tab.

Pull Requests Tab - GitHub

Select "New pull request"

New Pull Request - Github

When you're creating a pull request, use a title which describes the PR like you did with your commit.

PR - Fix login button not showing.

It can be useful to provide the reviewers with some context as to why this PR was necessary, or any additional info within the PR description.

As you can see above, I've chosen to include what the fix was, potentially making the review smoother. I've also included some important information regarding that this PR should not be merged until another PR is merged.

When working for some companies, they may require you to add a ticket reference to the PR title too, but I've already discussed why I think this is not needed.

PR Labels

If you want to make it even clearer, you can utilise PR Labels. These are labels which can be applied to the PR to illustrate either the state of the PR, or simple information to others. You can find them on the right of the pull request page:

You can select from pre-defined labels, or add your own.

• Click on Labels
• Enter the label you wish to use. In our scenario we're going to add a label called Do Not Merge.
• Press enter once you have typed in the value, and you can configure the label's colour and name. Once saved you can now type it in again and it will show within the list.
• Select your new label and voilà!

Click Create pull request

There you have it, you have created your first pull request.

How to Review a PR – Best practices

What to look for when reviewing a PR

Always take a step back and think about the key elements of a good code review. Here are some points to consider:

• Does the code follow your team's coding guidelines?
• Does the code meet its objective / acceptance criteria?
• Is the code legible and is it easy to understand what it's doing without the need for heavy comments / documentation? (This one for me is one of the most important, as I'm a huge fan of descriptive function and variable names.)
• Does the code need any refactoring, considering security, performance or simply ease of reading?
• Does the code follow simple design patterns / principles, like the single responsibility, abstraction, encapsulation, and so on? If not, you can make suggestions on how this can be achieved or perhaps teach those not familiar with it what it means and the benefits.
• Are there any "magic numbers / strings" that would be better served as a constant or named variable?

Review PRs in a timely manner

While you shouldn't necessarily feel obligated to look at a PR immediately, don't leave the author hanging, either. This PR could be blocking future work, or it could be important (if this is the case, the author should make this clear).

Try and keep discussions and PR comments flowing. If it makes it easier, you can jump on a call (if remote) or pull up a chair in the office and go through the PR together. This might speed up the process, and reduce the back and forth waiting around.

All this being said, don't rush the code review. Be meticulous, and read each file and change carefully. I advise you to pull down the branch the changes are made on, allowing yourself to look over the entire project, and not just the lines changed.

Many times, if you're looking at just one or two line code changes, you won't know exactly what the code is supposed to be doing. But if you look at the whole file you can follow the flow.

If you're using VS Code and GitHub, you can utilise their own extension for viewing pull requests and viewing comments whilst checking out the PR branch all within VS Code itself.

We're all human here

Remember that we're all human, and people often make mistakes when coding and can simply overlook things.

Not everyone codes the same way you do either, so don't simply request changes just because they've done it differently to how you would have. It doesn't mean that they've done it wrong, nor does it mean your way is the best.

Describe changes clearly and word your comments carefully

A pull request is not an exam, it is also not an opportunity for you to harshly criticise someone's work. It's a learning opportunity, and a chance to ensure the best code makes it into the main branch. It's about code quality and whether the code meets the acceptance criteria.

Think about the language you use when making suggestions. A PR comment is the opposite to a git commit message. No longer do we use the imperative voice. Do not command them to make changes, but instead make softer suggestions using the subjunctive mood instead.

As you're critiquing work that someone has most likely put a lot of effort into, use phrases like:

It it were me, I'd change this if statement to a switch case statement as it's more readable.

or

Perhaps consider renaming this variable to a more intuitive name, such as {alternative} to make it more understandable from initial read what it's doing.

As above, try and add your reasoning as to why you're making the change request. It will make the request more impactful, and allow the author to reflect on whether it would be best to make the change, or perhaps spark a discussion to come to a compromise.

Offer suggestions for improvement

Most Git systems allow you to click on the line you wish to be changed and add a comment so it's much simpler to specify the exact line of code you want changed.

Hosting providers such as GitHub have a "suggestion" feature which allows you to add a code suggestion directly into the comment, which can instantly be accepted and committed from within the PR.

If this isn't available, be sure to make sure what you're requesting is clear and concise. Perhaps even rewrite, or write out your suggestion within the comment such as:

Perhaps look at changing this if / else statement to a ternary statement like so:

var backgroundColor = isError ? 'red' : 'blue'

This makes the suggestion clear, and even speeds up the process of re-writing (using copy and paste).

Don't be afraid to defend your code

Remember a PR is a discussion. It's a two way process, giving you the opportunity to defend your code and explain with more context what you were thinking.

Just because the code may not look perfect, there may be a reason. There may have been matters out of your control which you had to contend with, forcing your hand to writing it in a certain way.

Don't be afraid to offer counter arguments, but be ready with valid reasoning, if you truly believe in your solution.

Communicate that the PR is approved

Many people nowadays filter their GitHub or version control email notifications to a folder which rarely gets looked at due to the volume of updates on repos, commits, branches and such.

To speed up the process, and make it more fun for the author, simply drop them a message. Many companies now offer some form of instant messaging service, so why not use it?

Setup a specific channel on your IM platform for PR's. Post in the channel / chat room your PR link, and allow reviewers to update you on their progress by replying within threads. Add an emoji to make it more light hearted (as we all know PR's can be boring, so why not spruce them up a bit).

Conclusion

In this article we've learned:

• How to setup VS Code with Git integration
• How to write useful Git commit messages
• When to commit to make it easier for the team
• How to effectively code review a PR
• How to handle PRs in a manner that helps all your team.

I hope you've learned something new, thanks for reading my article

Feel free to connect with me on Twitter @gWeaths

Code reviews bring many benefits to the process of writing and delivering software:

• Ensures consistency through your codebase.
• Teaches all members of the review (helps knowledge transfer).
• Builds contextual awareness regarding what might affect other parts of the team
• Helps avoid breaking builds
• Casts fresh eyes over a code change to search for optimisations and simplifications in the change.
• Promotes quality and helps make sure no one forgot or missed anything.

Google has 1,918 repositories on their GitHub across multiple languages, and even more that aren't open source.

One of their codebases is shared by over 25,000 Googlers, and typically has 40,000 commits daily (16,000 human changes, and 24,000 by automated systems). Each day it has to serve around 800,000 queries per second during peak times.

Google has published their engineering practices, so let's see how they review code at their scale, with so many commits a day.

Google refers to changes in their codebases as CLs standing for change lists. It's just a unit of work / piece of code going through review.

How to Prepare for Code Reviews

Firstly, before you even think about the code review process, try and focus on who will do the review. Try to pick a subject matter expert. Pick someone who is familiar with this codebase or area of the codebase.

Sometimes this may even mean having different people review different parts of the code. But less than 25% of code review's at Google have more than one reviewer.

Getting someone to respond in a timely manner is also important. To avoid bottle necks and overloading individuals, just CC reviewers onto the change to review it at their convenience if they want to.

How to Run Fast Code Reviews

Code reviews should be completed quickly. The maximum length of time for a review should be one business day.

Why the urgency? I've personally had QA's sometimes take weeks or longer.

• It becomes a blocker. Although the author of the code moves onto new work, new changes start to form a back log, and the delays can build up to weeks or months.
• Developers feel frustrated. If the reviewer asks for major changes but only responds every 3 days, it is frustrating for the developer working on that change. But with quick responses, whenever you require explanation of exactly what you need to do, the frustration fades away.
• Code quality can degrade. If your reviews are always slow, developers are less likely to do code clean ups, refactoring work or general code improvement ("If my reviewer won't reply for 4 days what's even the point?"), and the code quality submitted in the reviews is more likely to go down.

The main reason code reviews can be fast is because they're small. Rather than sending a 1,000 line CL, they're separated into multiple CL's – pushing 10 smaller changes of 100 lines for example.

Google also has emergencies where code changes have to be made quickly to resolve major problems in production. In these cases, code quality is relaxed. This review should immediately become the reviewers first priority. There are some examples of emergencies Google has faced here if you're curious.

Google's Code Review Standards

The key rule Google emphasizes is:

Reviewers should generally approve a change once it definitely improves the overall code health, even if it isn't perfect.

The key point Google seem to be making is that perfect code doesn't exist. If it makes it better, that is enough.

It's definitely a balancing act between something being better, and how much better it could be. If you could add more feedback to make significant improvements, there may need to be more work done on the code.

What Googlers Look for in Code Reviews

When doing code reviews, Google focuses on these elements according to their engineering practices documentation:

Design: Is the code well-designed and appropriate for your system? Does this change belong in your codebase, or in a library? Does it integrate well with the rest of your system?

Functionality: Does the code behave as the author likely intended? Is the way the code behaves good for its users? Mostly, we expect developers to test CLs well-enough that they work correctly by the time they get to code review.

Complexity: Could the code be made simpler? Would another developer be able to easily understand and use this code when they come across it in the future? Is it over-engineered for its current use case?

Tests: Does the code have correct and well-designed automated tests? Will the tests actually fail when the code is broken? Will they start producing false positives? Does each test make simple and useful assertions?

Naming: Did the developer choose clear names for variables, classes, methods, etc.?

Comments: Are the comments clear and useful? Ensure where sensible comments explain why something is being done, rather than how.

Style & Consistency: Does the code follow our style guides?

Documentation: Did the developer also update relevant documentation?

Google has style guides for multiple languages like C++, Swift, HTML/CSS, Lisp, JavaScript and more.

Having a document that everyone can refer to helps standardise the code. It also helps clarify what the expectations are in the review process.

How Googlers Review Code

There is a three-stage process for code review in Google's engineering practices. Here's a list of things you'd need to cover if you were to review:

1. Get a high level overview of the change
2. Examine the main parts of the change
3. Look through the rest of the code, in a sensible order

Let's go over each step in more detail.

1. Get a High Level Overview of the Code Change

Look at the code change's description/summary. Does it all make sense? For example, if someone is changing a codebase that is being deleted next week, push back on the change courteously, and explain why the change doesn't look needed anymore.

It's inefficient if people are spending time on work that isn't actually needed, so have a look at your development life cycle and see why this might be happening. The earlier you can catch these issues the better.

In order to get a high level overview of the code, you may want to briefly scan the code components to see how it all works together.

If you see a serious architectural problem or something majorly wrong, you should share your observations immediately at this stage. Even if you don't have time to review every single other element of the review. Reviewing the rest may well even end up being a waste of time if the architectural problems are severe enough.

Aside from this, there are 2 major reasons why you should send your review comments immediately:

• Googlers will often email a change, and then immediately start new work based on that change. If the change in review need serious adjustments, they may be building on something that needs to be significantly changed.
• Big changes to the CL may take a while to finish. Developers all have deadlines and it's courteous to try and help them meet them by letting them start rework as soon as possible.

2. Examine the Main Parts of the Code Change

Once you have had an overview, review the "main" parts of the change.

Find the file or files that are central to the CL. Often, there is one file that has the largest number of changes, and it’s the major piece of the CL.

Spend most of your focus reviewing these pieces. This provides context to all the smaller pieces, and generally makes it faster to review.

If the CL is too large for you to get a good overview and understand the flow, it's a good sign the developer should be splitting up their CL into smaller changes.

3. Look Through the Rest of the Code, in a Sensible Order

Once you have a good overview of the change, it's time to drill down into the details and go file by file.

Each reviewer generally does this differently. Some go in the order that the version control presents to them and some pick a particular order. Choose what makes sense to you. It's important to just review everything, and miss nothing.

Review Every Line of Code

Don't miss or skim important details. Sometimes there might be config files, or generated code you can scan briefly looking for irregularities. But your job here is to be very thorough and carefully scrutinise the code.

Make sure you think about bugs, race conditions, alternate approaches, conciseness, readability, and so on.

If you can't understand the code, it's highly likely other developers won't be able to either, and should ask the developer to clarify it.

Understand the Context

Often times the version control software will only show the changed lines. But it's important to read the lines before and after, or the whole file, to ensure you understand exactly what the change is doing.

You may only see someone adding 2 more if blocks in a change. But if you look at the context you might see that the if else block now has 15 different if's inside it. Then you can reject the code change and properly improve the code quality in this area, rather than making it worse.

Remember code degradation happens slowly, with every change that goes in. Our main aim is to ensure the quality trends consistently up, and we never go backwards.

How Googlers Write Code Feedback

Google has some specific sections covering how to do code reviews courteously, and respectfully. This isn't in opposition to being clear and as helpful as you can to the developer. The golden rule here is to make comments about the code and not the developer.

What to Do:

####

Ask for the why: If it is unclear why the author is doing things a certain way, feel free to ask why they made a particular change. Not knowing is OK, and asking “Why” leaves a written record that will help answer this question in the future. (And sometimes, “I'm curious, why did you decide to do it that way?” can help the author to rethink their decision.)

Find an end: If you like things neat, it’s tempting to go over a code review over and over until it’s perfect, dragging it out for longer than necessary. It’s soul-deadening for the recipient, though. Keep in mind that “LGTM” does not mean “I vouch my immortal soul this will never fail”, but “looks good to me”. If it looks good, move on. (That doesn’t mean you shouldn’t be thorough. It’s a judgment call.) And if there are bigger refactorings to be done, move them to a new CL. (Source)

What Not to Do

Don't use extreme or very negative language: Please don't say things like “no sane person would ever do this” or “this algorithm is terrible”, whether it’s about the change you're reviewing or about the surrounding code. While it might intimidate the reviewee into doing what you want, it’s not helpful in the long run - they will feel incapable, and there is not much info in there to help them improve. “This is a good start, but it could use some work” or “This needs some cleanup” are nicer ways of saying it. Discuss the code, not the person.

Don't bikeshed: Always ask yourself if this decision really matters in the long run, or if you're enforcing a subjective preference. It feels good to be right, but only one of the two participants can win that game. If it’s not important, agree to disagree, and move on. (Source)

Remember code reviews can sometimes trend towards finding issues or discussions with code and not praise.

If your reviewer has addressed your comments, or they have done some optimisation or smart code you like, thank them. Say you like the approach / how they have solved an issue.

Conversely, thank the reviewer if they thoroughly explain what they want and promptly answer your questions.

I have had some reviews where reviewers have put huge comments explaining something I misunderstood and I can still remember who it was and what they said.

How to Handle Pushback in Code Reviews

Sometimes the developer who is receiving the review might disagree with the changes that you're proposing. Here's how to handle that.

Consider That They Might Be Right

Sometimes the developer is closer to the code and how it works, and they may be right. If they are, move on from that discussion point and leave that code as it is.

But if they're not, or they've misunderstood something, you should insist on the change. Respond to what the developer initially challenged you on, and explain your reasoning courteously.

Code quality improvements tend to happen in small incremental steps, and review is one way it happens. Insist on the code improvement, even if it means extra work for the developer.

Try to Avoid "I'll fix this later..."

One of the most common issues in reviews is not that people disagree whether the code change is necessary, but rather that they want to do it under a different ticket or do it later. So if you pass this review, they assure you that they will follow up later and fix the issues.

Generally speaking, following up later rarely happens. This doesn't mean there's something wrong with the developer or their work ethic. But it's easy to forget, have priorities change, or even just get lost in their queue of work.

Insist that the work gets done now. There is no real fantastic gain to merging changes in the codebase you have to immediately fix. You are just adding technical debt that might be followed up on later, or might be forgotten. Fixing lots of tickets later is an easy way for quality to drop.

The only exception to this is emergencies, which involve the highest priority bugs Google deals with where there is something seriously wrong. This could be services going down, or errors crashing people's pages in production.

There is naturally an eagerness to merge changes ASAP, and the review might accept slightly lower quality, with a follow up fix immediately being written. The main point is that the first change removes the emergency, and the second change fixes it properly.

Conclusion

I hope this has explained some helpful concepts Google makes use of in their code review process. I wrote this to better cement it into my own mind, and was curious about how other companies handle the QA process.

I found that there are quite a few points that I can take away and apply to the reviews I conduct.

The document I referred to throughout this article can be found here.

I share my writing on Twitter if you enjoyed this article and want to see more.

When a project grows, and developers are pushing code frequently, there is always a chance that working pull requests might break somewhere.

It could be because one PR was merged before another, or the destination branch moved a few commits ahead, causing conflicts.

Or maybe because a developer didn’t run tests before pushing and unknowingly introduced a bug in some other part of the product. And the list goes on.

But this shouldn't be a problem. Every organization has a workflow for Code Reviews, right? But it still takes up a lot of time. Especially for those PR's that break and are not even ready for review.

We can manually build and test our code every time before a proper code review, no doubt. But after a certain point, it seems better to automate it.

Imagine a medium size organization with 100–150 PR’s every week. The time spent repetitively validating those might give that firm a whole set of new features. Well then, let’s go get those features!

Prerequisites

You should have some familiarity with AWS Services.

I assume you know how to create and manage Lambda Functions, Codebuild Projects, Cloudwatch Events, IAM Roles, and that you’re using CodeCommit to version your codebase.

Architecture

Let’s understand, at a high level, how we are going to tackle this project.

Uh huh, What?

Step by step, let’s understand our workflow better.

1. Let’s say a new PR is created / an existing PR is updated.
2. A Cloudwatch event that is watching our repository will be activated and will send relevant data to a lambda function.
3. This function will do two things
   → Trigger CodeBuild Project to build our latest commit and run tests.
   → Comment any custom message that we want on our PR.
4. After CodeBuild finishes running the build, another Cloudwatch event will send those build results to a lambda function.
5. This function will comment the build results on our PR.

Alright then, let’s get started!

Setting up our App

For the sake of simplicity, I’ve created a simple Node.js application. It’s written in TypeScript and all the build phase does is compile the ‘app.ts’ to ‘app.js’.

Here’s the link to the repo – clone and use it if you want to follow along.
All the relevant code used in this article can be found there.

A simple Express app

The build command here is a simple tsc app.ts , but you can change it to your project’s build command.

Also to keep it simple, I’ve not included test cases. You can link them to test in the script section of package.json and follow along.

Codebuild Project

First, you'll want to set up a basic CodeBuild project for your repository.

To do this, do the following:

• Set up source as your Codecommit repository
• Reference type should be branch
• Environment should be per the project's requirements
• You should use a buildspec file
• The rest should just be defaults.

Make sure you have a buildspec.yml file in your repo’s root folder.

Note: this might differ if you’re dealing with a MonoRepo. In that case, you might have separate buildspec.yml files for each App and will have to selectively pass the buildspec file path as an environment variable depending on the files changed inside the commit.

We have a similar setup at our organization, and we're loving the results at present!

version: 0.2
phases:
  install:
    commands:
     - n 12.12
     - curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add -
     - echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list
     - apt update
     - apt install yarn
     - yarn install
#   pre_build:
#     commands:
#     - yarn test
  build:
    commands:
     - yarn build

What does this buildspec.yml do? Well, it passes runtime commands for each build to our CodeBuild project.

And then does what? ?

• Installs node 12.12.0
• Installs yarn
• Installs our project’s dependencies.
• yarn test (It runs our test cases. There are none here, but you can uncomment that section if you need it.)
• yarn build (Builds our project.)

Lambda Functions

Let’s set up two functions as discussed in the architecture section above.

The function TriggerCodebuildStart will receive a Cloudwatch Event (which we will set up in a bit) and it will trigger our CodeBuild project to start a fresh build.

It will also post a Build Started comment with the timestamp and a neat link to the build logs in our PR’s comment section.

The function TriggerCodebuildResult will receive a Cloudwatch Event from our CodeBuild project which will have the build results.

It will also post the Codebuild Results comment with the timestamp and a neat link to the build logs in our PR’s comment section.

Here’s the code. That’s what you were waiting for, weren’t you! ?

const AWS = require('aws-sdk');
const codecommit = new AWS.CodeCommit();
const codebuild = new AWS.CodeBuild();

exports.handler = async (event) => {
    try {
        console.log('Received Event: ', event);
        const { destinationCommit } = event.detail;
        const { sourceCommit } = event.detail;
        const { pullRequestId } = event.detail;
        const pullRequestName = event.detail.title;
        const sourceBranch = event.detail.sourceReference.split('/').pop();
        const triggerCodeBuildParameters = {
            sourceBranch, sourceCommit, destinationCommit, pullRequestId, pullRequestName
        };
        const codeBuildResult = await triggerCodebuild(triggerCodeBuildParameters);

        const buildId = codeBuildResult.build.id;
        const postBuildStartedCommentOnPRParameters = {
            sourceCommit, destinationCommit, pullRequestId, buildId
        }

        await postBuildStartedCommentOnPR(postBuildStartedCommentOnPRParameters);

        return {
            statusCode: 200
        };
    }
    catch (error) {
        console.log('An Error Occured', error);
        return { 
            error
        };
    }
};

async function postBuildStartedCommentOnPR(postBuildStartedCommentOnPRParameters) {
    const { sourceCommit, destinationCommit, pullRequestId, buildId } = postBuildStartedCommentOnPRParameters;
    const logLink = `https://${process.env.REGION}.console.aws.amazon.com/codesuite/codebuild/projects/ValidatePullRequest/build/${buildId}`;
    const parameters = {
        afterCommitId: sourceCommit,
        beforeCommitId: destinationCommit,
        content: `Build For Validating The Pull Request has been started.   
        Timestamp: **${Date.now()}**   
        Check [CodeBuild Logs](${logLink})`,
        pullRequestId,
        repositoryName: process.env.REPOSITORY_NAME
    };

    const request = await codecommit.postCommentForPullRequest(parameters);
    const promise = request.promise();
    return promise.then(
        (data) => data,
        (error) => {
            console.log('Error In Commenting To Pull Request', error);
            throw new Error(error);
        }
    );
}

async function triggerCodebuild(triggerCodeBuildParameters) {
    const { sourceBranch, sourceCommit, destinationCommit, pullRequestId, pullRequestName } = triggerCodeBuildParameters;
    console.log(`Triggering Codebuild, Branch: ${sourceBranch}`);
    const parameters = {
        projectName: process.env.CODEBUILD_PROJECT,
        sourceVersion: `refs/heads/${sourceBranch}^{${sourceCommit}}`,
        environmentVariablesOverride: [
            {
                name: 'pullRequestId',
                value: pullRequestId,
                type: 'PLAINTEXT'
            },
            {
                name: 'sourceCommit',
                value: sourceCommit,
                type: 'PLAINTEXT'
            },
            {
                name: 'destinationCommit',
                value: destinationCommit,
                type: 'PLAINTEXT'
            },
            {
                name: 'pullRequestName',
                value: pullRequestName,
                type: 'PLAINTEXT'
            }
        ]
    };
    const request = await codebuild.startBuild(parameters);
    const promise = request.promise();
    return promise.then(
        (data) => data,
        (error) => {
            console.log('Error In Starting Codebuild', error);
            throw new Error(error);
        }
    );
}

const AWS = require('aws-sdk');
const codecommit = new AWS.CodeCommit();
exports.handler = async (event) => {
    try {
        console.log('Event', event);
        const parameters = await getParameters(event);
        console.log('Parameters For Comment:', parameters);
        await commentCodeBuildResultOnPR(parameters);
        return { statusCode: 200 };
    }
    catch (error) {
        console.log('An Error Occured', error);
        return { error };
    }
};

async function getParameters(event) {
    try {
        const buildId = event.detail['build-id'].split('/')[1];
        const buildStatus = event.detail['build-status'];
        const environmentVariableList = event.detail['additional-information'].environment['environment-variables'];
        let afterCommitId, beforeCommitId, content, pullRequestId;
        for (element of environmentVariableList) {
            if (element.name === 'pullRequestId') pullRequestId = element.value;
            if (element.name === 'sourceCommit') afterCommitId = element.value;
            if (element.name === 'destinationCommit') beforeCommitId = element.value;
            if (element.name === 'pullRequestName') pullRequestName = element.value;
        }

        const logLink = `https://${process.env.REGION}.console.aws.amazon.com/codesuite/codebuild/projects/ValidatePullRequest/build/${buildId}`;
        content = `Build Result: **${buildStatus}**   
        Timestamp: **${Date.now()}**   
        Check [CodeBuild Logs](${logLink})`;

        return {
            afterCommitId,
            beforeCommitId,
            content,
            pullRequestId,
            repositoryName: process.env.REPOSITORY_NAME
        };
    } catch (error) {
        throw error;
    }
}

async function commentCodeBuildResultOnPR(parameters) {
    const request = await codecommit.postCommentForPullRequest(parameters);
    const promise = request.promise();
    return promise.then(
        (data) => data,
        (error) => {
            console.log('Error In Commenting To Pull Request', error);
            throw new Error(error);
        }
    );
}

You'll need to fill in the appropriate environment variables before using these functions. Read the code once and you’ll know what to do.

In case you need to refer to the docs, just go here and there. ?

Configure Cloudwatch Events

Okay, now to hook it all up, let’s configure our Cloudwatch events.

We’ll create two events: One will receive new commit data from our repository, and the other will receive the Codebuild Results. The targets for these events will be our lambda functions.

I'm attaching full-page screenshots here. This will make it easier for you to understand the references.

Focus on the Green Ones

Replace with your CodeBuild project’s ARN.

Almost there!

I’ve chosen to trigger the lambda function on FAILED and SUCCEEDED events. But you can select All Events too and tailor it to your needs.

And, Action!

Okay, you’re super cool if you made it to this point. ? After so much work, Let’s see what we’ve achieved.

Let’s make two pull requests, one which is works well and the other which has an intentional build error.

Error Free PR

Great!

Now, let's create a PR with bugs. See here, instead of app.get there’s ap.get. It’s intentional and silly. But it will do the job for now.

Faulty PR

Failed Build Message, Happy reviewers. Didn’t have to checkout the branch and test!

Devs, as usual, we’ve got logs for you!

Wrapping up

To take this a step further, you could trigger an API call to your Slack webhook URL to immediately notify in a channel in case of build failures. Awesome, right?

Also, this is a very simple set up and real-world projects might be more complex.
For example, MonoRepos might have multiple apps and builds, and tests for each of those apps are different.

Triggering all those tests every time will be of no use, and it'll be costlier and create confusion. You might need to selectively trigger those builds depending on which files were committed and which apps were affected.

However, this article should build up a decent base for you. And you can definitely expand on it. After all, you’re awesome too. :)

Thanks for reading! If you need some help regarding this, feel free to reach out to me on LinkedIn. Looking forward to helping however I can.

Writing a good Pull Request description is one of the most tedious jobs a developer has to do. And it can feel counter-productive at times.

But developing this skill goes a long way and really helps your stakeholders and your organization in the long run.

So it's always better to put in those 10 extra minutes today rather than having to break your head 6 months down the road trying to explain why you did what you did.

The following article explains the different parts of a Pull Request Description, and why you should include them.

What is a PR description?

A pull request description describes what constitutes the Pull Request and what changes you have made to the code.

It explains what you've done, including any code changes, configuration changes, migrations included, new APIs introduced, changes made to old APIs, any new workers/crons introduced in the system, copy changes, and so on. You get the gist.

You should include this section because it gives a glimpse to different stakeholders into what the PR is doing.

For a non-technical person, the description should explain what the impact will be on the system when these code changes are deployed to production.

It will also help the reviewer in understanding what they will be reviewing (kind of a prologue/trailer).

And finally it helps QA/SDET in understanding the scope of the PR as well.

So the "what" of the PR should give a glimpse into what constitutes the changes in the PR.

The "Why" section

This section explains why the above changes explained were done.

Sometimes a developer feels that it's okay to write "Business/Product requirement" in the description. That's fine, but doing so defeats the purpose of this section.

If there is a better explanation as to why the changes were suggested, it's always good to attach a document reference link for that information. A good "Why" section should explain the reasoning behind any changes.

You should include this section because it gives an explanation of why you suggested your changes. It might not sound reasonable to include this section in the shorter term, but it plays a vital role as the product matures and spans years.

Developers/Product folks comes and goes, but the code remains. And when a new developer works on a piece of code and finds a discrepancy there, they can dig deeper and get to the pull request from 2 years ago which made those changes.

A good "why" gives them the explanation and assumptions made at that time.

GoJek's CTO once explained that they don't challenge their legacy code and don't question why there seems to be some absurd looking feature in the product.

They just go back and check the documentation.

The assumptions and circumstances might have changed, and hence the code evolves. What seems to be quite absurd today might have been relevant 2 years ago. So it's better to explain today why you are making a particular change.

Testing Scope

This section should comprise a list of scenarios you need to keep an eye on when testing this particular PR. (This will include flows, APIs, crons, workers, and so on.)

A good testing scope gives visibility to the QA/SDET team about the scenarios and flows that they need to test.

It can also help you while you're writing this section. I have come across issues myself, which prompted me to go back to the development phase and test them again.

A thoroughly written Testing Scope helps developers test the PR more efficiently, and gives a thorough picture of the PR to the person testing it.

Relevant Document(s)

This section includes any relevant document that needs to be attached with the PR description.

That might include product requirement documents, architectural diagrams, database system diagrams, design patterns used, class diagrams, external library documentation, and so on.

This section helps explain any assumptions and references you made while working on this feature request. And it plays a major role in the long run.

When someone comes back and sees why such a change was suggested, the relevant documents section will take them to the specific documentation so they can understand the issue clearly. Or it can take them to the technical implementation details of the scenario at the time of development.

Dependent PR(s) (if any)

There are times when a particular feature spans across multiple GitHub repositories and it's important to release them in a certain order.

For example, you might need to deploy one repo prior to another, or you might need to deploy them side by side.

Whatever the case may be, this section explains any dependent code that this PR relies upon.

You should include this section because it really helps the deployer understand the order of deployment. If another repo's code needs to go first, then the deployer will contact the person responsible for deployment of the other repo to make sure that their deployment happens first. Overall it helps smooth out the deployment process.

Configuration Changes (if any)

This section should include all the config changes that need to be added to the secrets before the PR is deployed into production.

There will be times when the deployer will merge 10-20 PRs at a time and it becomes hard for them to keep track of all the config changes.

Because of this, it's always better to include them in the config changes section. (Don't put the secret keys there, just include the key names and how to get the secrets.)

Tags/Labels

These are very important in a pull request description and convey a lot of meaning when the team grows.

Following are some tags that I use, but you can add different tags according to your needs.

• Dev Completed - When the development is completed from the dev's side.
• Self Reviewed - When the developer(s) of the Pull Request has reviewed the Pull Request from their side, and can now give it to their peers for a Peer Review.
• Self Tested - When the developer(s) of the Pull Request has tested the changes themself according to the description they have given in the Testing Scope section.
• Config Changes - This tag indicates that there are some configuration changes that need to be done before deploying the PR to production. (This includes secret keys that needs to be put into the system.)
• Contains Migration(s) - This indicates that the PR contains a Migration. If it is a long running migration, it should be specified beforehand.
• Release Ready - This indicates to the Deployer that the PR is ready for deployment and will be picked up by the deployer in the next deployment cycle.
• Peer Reviewed - When the Peer has reviewed the PR and the suggested changes are made by the developer(s). This is put up by the Peer who has reviewed the PR.
• QA Tested - This indicates that the QA/SDET has tested the PR and it is good to be deployed to production.

Conclusion

In this article, we went through the different parts of a PR description. Most of the PRs you make will not need all these sections and tags, but you should try to include as many of them as you can.

Writing this description might not seem productive at the time when you create a PR, but it can definitely prove helpful in the future.

A final note: The above are my views and might differ from your perspective. But I've developed this strategy over the years, and it contains the input and experience of a lot of people I have worked with in the Industry. So you can feel good knowing that it's been battle tested to a high degree.

In this article we'll explore what kinds of things you can do to make it a little easier on your future self.

Revisiting "prior art"

We've all been there. Six months into a project, you're trying to squash a bug, and what you find is shocking. You might be asking yourself, "who would write this kind of code?"

So you dig through your git commit history using git log -p filename.js showing changes for a specific file, trying to see who would come up with something like that. And then your heart drops – you're the one who wrote it!

Tituss Burgess shocked

This is a common scenario for any developer, experienced or new. If you haven't hit that scenario, I promise if you stick with coding long enough, you will.

Becoming more conscious about our coding habits

That six month reflection point is inevitable. That's a lot of time which you've most likely been using to work on other parts of the project or another project completely. Chances are, you've leveled up which has changed the way you write code.

On the other hand, sometimes it takes stepping outside of the code to see the bigger picture and get a better look at how all the pieces fit together. We can naturally dig ourselves too deep into a solution and can become a little narrowly focused as we work to solve those challenges.

But either way, while part of the code journey will simply be gaining more experience and learning more about your craft, there are other small habits you can get used to early on that will help you down the road.

So let's jump in.

Improving the readability of your code

What is the challenge?

Part of the fun about our craft is there are a ton of ways you can do the same thing. Think an if statement is too many lines? Well we can write it ternary style!

// Example ternary statement
const isFreezing = temperature <= 32 ? true : false;

But sometimes this takes a toll on the readability of your code. While it might seem like it looks nice and super clean on one line, imagine that as that ternary gets more complex, someone will have to spend more time understanding what it means.

const minutes = 30;
const cookie = {
  color: 'black'
};

const cookieStatus = minutes > 20 ? cookie.color === 'black' ? 'burned' : 'done' : 'not done';

What can we do better?

Now I would imagine most of us can figure out what cookieStatus is in this example (spoilers: burned). But think about the time you spent figuring it out. Whether an extra 1s or 2s, it forces you to spend additional cognitive energy to read through the code.

On the other hand:

const minutes = 30;
const cookie = {
  color: 'black'
};
let cookieStatus;

if ( minutes <= 20 ) {
  cookieStatus = 'not done';
} else if ( cookie.color === 'black' ) {
  cookieStatus = 'burned';
} else {
  cookieStatus = 'done';
}

No, it may not be as clean or clever as the 1-line ternary statement, but the next time you visit it, the less you'll have to think about what the answer is.

It's also going to make it that much easier for bugs to creep up and get past your code reviewers when all of your code changes are in a 1-line git diff.

And yes, this is a simple example. But imagine this in a real world scenario with important business logic where you could run into these situations frequently.

Say you need to add another condition – that ternary is just going to keep getting more complex! You're just making it more difficult to debug or extend, where the if statements can continue on in an easily readable way.

For what it's worth, ternaries and other shortcuts can be simple and effective in code, but don't abuse that effectiveness and end up making things more difficult.

Keeping things consistent

What is the challenge?

We all have our favorite way to code. Though I'd argue not including a semicolon at the end of your JavaScript is just completely wrong, you might prefer to write your code the wrong way without them.

// Jim's code style

function MyComponent() {
  function handleOnClick() {
    alert('Click!')
  }
  return (
    <button onClick={handleOnClick}>My Button</button>
  )
}

// Creed's code style

const MyComponent = () => <button onClick={() => alert('Click!')}>My Button</button>;

But it's not always about what you prefer. When working with a team, chances are, everyone's opinion on how code should look is slightly different. You might agree about the semi-colon, but disagree about whitespace.

And no one is wrong (except for the non-semi-coloners)! There are valid arguments to most code styles, whether for or against, but the solution isn't for everyone to write their code their own way.

What can we do better?

Keeping code consistent is important to maintain code health. A typical goal is to "make the codebase look like one person wrote it."

The point isn't that one person gets their way, it's that the team came to a conclusion about a set of rules they would use that everyone would follow. Having this consistency provides less cognitive overhead as people work through the code. It gives everyone the ability to know what to expect when reading the code.

Errors from linting code

And achieving this doesn't need to be hard. There are tools that can simply check for these inconsistencies like Eslint for Javascript. And even better, there is another level of tools like Prettier that will fix it for you!

Commenting your code

What is the challenge?

Keeping up with commenting your code is an important way to put context to complex logic. As much as we all want our code to be self-documenting, that's rarely the case.

Too often we find ourselves dealing with a block of code that just doesn't make sense. And even if it makes sense on its own, we might not be able to figure out how it fits in to the rest of the application.

What can we do better?

By providing a good set of comments, you're setting up the next person who touches that code to have a better understanding of what the code is doing before they make a change.

// DONT CHANGE - WILL STOP MAKING MONEY

const shouldMakeMoney = true;

function makeMoney() {
  if ( shouldMakeMoney ) {
    return noMoney;
  }
  return moreMoney;
}

While this is a silly example, it brings up a real world case. Businesses are increasingly dependent on being able to maintain a reliable website to make money. Whether this is as an ecommerce business or an ad giant, these websites rely on business logic that determine things like cost, taxes, discounts, and other math related things we tend to not want to think about, but could make or break a business on the internet.

But it's not all about the company you work for. Touching old code can be scary. It's even scarier when no one on your team was around when it was written, so no one knows what it does!

Patton Oswalt hands over mouth

While you might not be the next person who touches that code, try to help out your future friend who's tackling the next ticket involving that code. Because there's also a good chance you will be that person and you'll wish you remembered how it worked.

Documenting your solutions

What is the challenge?

Documentation is similar to commenting your code, but from a different perspective. Documentation and commenting are both about finding ways to describe a solution in a human-readable way that will ultimately give more context. But documentation is more about the overall solution rather than the implementation details.

Having a high performing application is everyone's goal. But how did we get there? There's a realistic chance that someone will have to be working on the same project you are like onboarding a new team member. How will they be able to maintain that high performance if they don't know how it works?

What can we do better?

Whether it's introducing that new team member to the project or trying to share knowledge with another project team, documentation is an important part of maintaining a project. It helps keep everyone on the same page so we all confidently know what we're working towards.

For example, if we're still working with our ecommerce project with our business logic, there will be rules that the code needs to implement. While commenting might give details inline about how the rules were implemented, the documentation would define those rules.

/**
 * DOCUMENTATION
 * Order Total >= 25: Discount %10
 * Order Total >= 50: Discount %15
 * Order Total >= 100: Discount %20
 * Order Total >= 75: Free Shipping
 */

const orderSubTotal = 84.00;
let orderTotal = orderSubTotal;

// If the order total is under 75, apply shipping discount

if ( orderTotal < 75 ) {
  orderTotal = addShipping(orderTotal);
}

// If the order total reaches a threshold, apply given discount

if ( orderTotal >= 100) {
  orderTotal = applyDiscount(orderTotal, .2);
} else if ( orderTotal >= 50 ) {
  orderTotal = applyDiscount(orderTotal, .15);
} else if ( orderTotal >= 25 ) {
  orderTotal = applyDiscount(orderTotal, .1);
}

This is a minimal example, but we can see the difference between the rules at the top and how we apply them. The documentation should clearly explain what the rules are, but it shouldn't care about how those rules were implemented.

On the other hand, the comments might not care about what the rules are, but need to implement them in an efficient and logical way. We should be able to update the code with the business rules, such as changing the top level discount tier from $100 to $80, without having to rework the code.

But documentation is much more than business rules – it's providing a way for anyone to understand your work from a higher level. This could include anything from architectural diagrams to the theory behind your core algorithm.

While maybe code isn't the best place for details like this to live, it's really important information that can help instill confidence in your project and give others an opportunity to understand more about the work.

Creating effective Pull Requests

What is the challenge?

Pull requests (or merge requests) are a core part of any development team's project lifecycle. It provides a way to package and present your code in a consumable way for your peers to review and understand your work.

There's a lot that can go into a pull request from a single commit to the entirety of the next version of your website. That's a lot of context to expect someone to understand by reading through the commits alone.

What can we do better?

Pull requests don't need to be an art. There should be one primary goal of the preparation you put into it – providing context into your changes. At a minimum, it should answer the questions of "what" and "why".

We can even use tools like pull request templates to push us in the right direction. Define an outline of what you want explained and chances are, people will follow that outline. This helps avoid the 1-line "closes [ticket]" description or even worse, an empty description.

With my projects, I hope to have a few questions answered before I dive into a code review:

• What is the change?
• What does it impact?
• How do I reproduce or test the change?

Just a few details around the change set can provide much needed context for those reviewing your code. It's easy to look at code, but it's harder to understand it without knowing how it fits into the bigger picture.

Hardening your code with tests

What is the challenge?

Tests are a way to ensure your code runs the same way each time. Being able to prove that the same input will always have the same output will help provide you and your team with a higher level of confidence that your application won't come crashing down with the next small change.

Without them, we're left with human error, where no matter how good your QA Engineer is (shoutout to my testers out there), something will always slip through. And that's not to say your tests will always catch every problem, but we can use the tools available to help prevent it.

What can we do better?

Where comments are a way of providing the context of how something works, test are a way to ensure they work. Providing test cases that are repeatable helps enforce that.

function applyDiscount(value, discount) {
  const discountAmount = value * discount;
  return value - discountAmount;
}

expect(applyDiscount(10, .1)).toEqual(.9);
expect(applyDiscount(532151235, .1054)).toEqual(476062494.831);

If I fudge the math on our applyDiscount function above, there's a high probability that the test would fail (never say never).

But testing doesn't need to be hard. There are many tools out there that help from different perspectives. For example, you might use Jest to run your unit tests or add Enzyme on top to test your React components. But you can also bring in Cypress as an integration test solution that will work like a robot clicking through your application to make sure all the components actually work together.

There's also different methodologies of testing. While you probably see most teams write their tests after they have a working solution, some people swear by test-driven development. They might write their tests first where the code has to pass the tests rather than the other way around. This is a great way to define the requirements of the code before diving right in.

Whatever the method, capture the points that are most likely to break or the functions that add the most business value. You'll be helping to prevent a potential business loss or even simpler, a headache.

What can we learn from this?

That might be a lot to digest, but they're important points to consider as you grow as a developer. Starting these habits early in your career will help you naturally build these skills and work that way by default.

And if you're late in your career, it's never too late to start. We should all want to strive to be the best developer we can be and do our best to help make our teammate's lives easier, as we're all in this together.

Looking for more to learn?

• Put Down the Javascript - Learn HTML & CSS
• How to Become a Full Stack Web Developer in 2020
• What is the JAMstack and how do I get started?
• What is linting and how can it save you time?
• Why you should write merge requests like you’re posting to Instagram

What’s your advice for growing as a developer?

Share with me on Twitter!

Previously I wrote about linting, what it is, and how it makes your life easier. At the end, I actually included a way that you could automatically fix your code. So why am I writing this?

What do you mean fix it?

Before we roll into it, let’s hit this quick. Linters are powerful and provide an easy way to scan your code for syntax errors that could lead to bugs. Or they can simply help keep a codebase clean, healthy, and consistent. When run, it will show all the issues and let you go through each one individually to fix them.

Taking that to the next level, some linters will actually allow you to pass in an argument to the command running the linter that allows it to fix it for you automagically. This means you don’t have to manually go through and make all of those little whitespace and semicolon (add them! ?) tweaks yourself!

Ron Swanson happy

So what more can I do to fix things?

If you already use the fix option, thats a good start. But there are tools out there that have been developed specifically to tackle this problem beyond just a flag into your command. The one I’m going to cover is Prettier.

What is Prettier?

Prettier pegs itself as “an opinionated code formatter." It takes an input of your code and outputs it in a consistent format stripping any of the original code style. It actually converts your code to a syntax tree, then rewrites it using the styles and rules you and Prettier provide together via your ESLint config and Prettier’s default rules.

You can easily use Prettier alone just to format your code, which works just fine. But if you combine this with an underlying ESLint process, you get both a powerful linter and a powerful fixer. I’m going to show you how to make those work together.

Voltron

Getting started with Prettier

For this walkthrough, I’m going to assume that you have ESLint set up and configured in an application. Particularly, I’m going to pick up where I left off in my previous walkthrough where we installed ESLint to a React application.

Additionally of note, Prettier tells us right from the start that it's an opinionated code formatter. You should expect that it will format your code in a consistent way, but maybe a different way than you currently have it configured. But don’t fret! You can tweak this configuration.

So what are we starting off with? We already:

• Have installed ESLint
• Have added Babel as our parser
• Have added a plugin that includes React configurations

Next, let’s get started by installing a few packages:

yarn add prettier prettier-eslint prettier-eslint-cli -D

Note: the command above is similar to using npm. If your project doesn't use yarn, swap out to npm as appropriate.

Above, we’re installing:

• prettier: core Prettier package and engine
• prettier-lint: passes the Prettier result to ESLint to fix using your ESLint config
• prettier-eslint-cli: helps Prettier and ESLint work together on various files across your project

And we’re installing them as a dev dependency, as we don’t need it outside development.

Configuring your new formatter

Now that our packages are installed, we can set up yarn to run this command for us.

Previously, we set up a lint script to look like this in our package.json:

"scripts": {
  ...
  "lint": "eslint . --ext .js"
  ...
}

We’re going to leave that as it is, but we’ll do something similar and create a new script right next to it called format for our formatter Prettier:

"scripts": {
  ...
  "format": "prettier-eslint --eslint-config-path ./.eslintrc.js --write '**/*.js'",
  "lint": "eslint . --ext .js"
  ...
}

So what’s going on there? We’re:

• Adding a new script called format, that we can run as yarn format
• We’re utilizing our prettier-eslint-cli package to run the formatting for us
• We’re passing in our ESLint config located next to our package.json in the root of the project (change this if it’s in a different location)
• And finally, we’re telling prettier to write all files matching **/*.js, or any JS files it finds recursively through our project

The beauty here is that we're passing in our ESLint config to Prettier. This means we only have to maintain 1 config for both tools, but we still leverage the linting power of ESLint along with the formatting power of Prettier.

Run your formatter!

Now that we’re all set up, let’s run it! Run this following:

yarn format

and immediately, we see that it works:

Successfully running Prettier

Hey, my code looks different!

Angry mob with pitchforks

As I mentioned earlier, Prettier tells us straight up, it’s an opinionated formatter. It ships with its own rules, sort of like its own ESLint config, so it will go through and make those changes as well.

Don’t abandon your code! Instead, you can review the changes, see if maybe it makes sense to keep it that way (it will be very consistent) or you can update your ESLint config (.eslintrc.js) to overwrite the rules you don’t like. This is also a good way to maybe learn some new things that you might not have expected to get caught before.

So where does this leave us?

If you’ve followed along so far, we now have two commands:

• lint: which will check your code for you and tell you what's wrong
• format: will automatically try to fix the problems for you

When using these in practice, your best bet is to always run format first to let it try to automatically fix anything it can. Then immediately run lint to catch anything Prettier wasn’t able to fix automatically.

What’s next?

Now that we can format our code automatically, we should be able to fix our code automatically!

Eddie from Fresh Off the Boat's mind blown

Next time we’ll take this a step further and set up a git hook that will allow this to run before you commit. This means you won't ever have to worry about forgetting to run this again!

Originally published at https://www.colbyfayock.com/2019/11/dont-just-lint-your-code-fix-it-with-prettier/

So what is linting?

lint, or a linter, is a tool that analyzes source code to flag programming errors, bugs, stylistic errors, and suspicious constructs. https://en.wikipedia.org/wiki/Lint(software)

Simply put, a linter is a tool that programmatically scans your code with the goal of finding issues that can lead to bugs or inconsistencies with code health and style. Some can even help fix them for you!

Michael Scott - Tell me more

Take for instance, the following example:

const test = 'I am a test';
console.log(`Test: ${test}`);
const test = 'Another one.';

We’re declaring the constant test twice, which our javascript engine won’t be happy about. With the proper linter settings and watch configuration, instead of getting caught later as an error when the code runs, you’ll immediately get an error through your linter running in the background:

  10:9  error  Parsing error: Identifier 'test' has already been declared

   8 |   const test = 'I am a test';
   9 |   console.log(`Test: ${2}`);
> 10 |   const test = 'Another one.';
     |         ^

It might be pretty obvious that you have 2 of the same const declarations given this is only 3 lines, but in a more complex application, this can save tons of time having to hunt down a pesky bug that’s not always obvious.

What all can linting help with?

Lots of things, including but not limited to:

• Flagging bugs in your code from syntax errors
• Giving you warnings when code may not be intuitive
• Providing suggestions for common best practices
• Keeping track of TODO’s and FIXME’s
• Keeping a consistent code style

Most things you can think of probably already exist in one form or another, and if not, you can even create custom rules that fit your needs!

How is this actually helping or why should I care?

Probably the biggest overlying theme of the list above is the fact that these issues will be called out immediately. No longer will these issues creep up on you in the middle of running your app or give someone anxiety during a code review. No longer will you and your reviewer endlessly fight passive aggressively through the comments about whether or not to include a semicolon at the end of JS statements (you should ?).

Grandma looking for a semicolon

All of those moments that stop you from being productive because of a silly syntax error or the micro-interactions you and your teammates have during a review take time. They add up and end up taking away the time you can spend fixing another bug or developing the next great feature of your product.

So how do I actually get started?

Even though there are linters for most, if not all, other mainstream languages, for the purpose of this post, I’m going to focus on Javascript. The same principles apply, but the tooling may be a bit different.

I’m going to run through how you can get set up for basic linting in a React app. You can easily follow along by spinning up your own React app or using my Gatsby starter: https://github.com/colbyfayock/gatsby-starter-sass#starting-from-scratch

Your Linter

To get started, we first need a linter. Probably the most popular in the Javascript world is ESLint. Your linter will actually be the engine for defining rules and parsing your files to test against. ESLint is available as an npm package by itself and once installed, out of the box it allows you to set up a basic configuration file and hit the ground running with some command line tools.

Let’s first add our ESLint dependency:

yarn add eslint -D

We’re installing this as a devDependency (hence the -D flag), because this isn’t something our application needs to run. After it’s successfully installed, let’s add it to our package.json as a script:

...
"scripts": {
  ...
  "lint": "eslint .  --ext .js"
  ...
},
...

In the above, we’re running our linter on the entire project directory on any file that has an extension of .js. If you're working with a large project with many file types, maybe even some you don't want linted, you can change that flag or be more specific with other options.

To support ESLint, we’ll need to do one more thing. Let’s add a file at the root of our project (probably where your package.json is) called .eslintrc.js and make the contents of the file simply:

module.exports = {};

Once you’re ready, you can run yarn lint and… get an error.

Lint results - Import errors

This is okay, and expected in our project, so let’s move on.

Your Parser

A common tool in the chain for Javascript developers is Babel, which allows you to write code with features that may not be supported in all browsers, such as using arrow functions, that are available in ES6, and other conventions like importing files via import.

The code you write may already run through Babel to work in a browser, but that doesn’t apply to ESLint by default, so ESLint allows you to specify a parser that allows the linting processing to look at the same code as your browser sees. In this case we’ll want to use Babel’s ESLint parser that’s made available to us.

To set that up, we’ll want to first install our dependency:

yarn add babel-eslint -D

Typically if you're using babel-eslint you'll want to make sure babel is installed next to it, but in our case, Gatsby already uses babel, so we don’t necessarily need to add it. After that’s set up, we’ll want to update our .eslintrc.js config file with some new options:

module.exports = {
    "env": {
        "browser": true,
        "node": true,
        "es6": true
    },
    "parser": "babel-eslint"
};

Here, we’re letting ESLint know that our environment will be run in node (Gatsby’s precompiling), inside the browser (the app), and it will use ES6. This helps ESLint know how to run your code. Additionally, we want to set up our parser to be babel-eslint.

Once we’re ready to go, run yarn lint again and… well nothing really happened.

Lint results - Nothing happened

This is still expected, as we don’t have any rules set up!

Plugins for your code

Writing a React app? The Babel parser may help you transform your code, but you might have a hard time being productive, as ESLint needs to understand how it should work to lint your React files.

Part of the beauty of ESLint is that it allows you to configure plugins that have the opportunity to create and set rules for you. Luckily, along with our Babel parser above that does some of the heavy lifting, we have a React plugin available that does just that and takes care of linting the JSX for us.

Let’s first install our dependency:

yarn add eslint-plugin-react -D

Further, let’s update our .eslintrc.js file again:

module.exports = {
    "settings": {
        "react": {
            "version": "detect"
        }
    },
    "env": {
        "browser": true,
        "node": true,
        "es6": true
    },
    "plugins": [
        "react"
    ],
    "parser": "babel-eslint"
};

What we’re adding here is a setting that automatically detects what React version you’re using, which is helpful to let your linting get parsed properly, and then set up our react plugin that we installed above.

For one last final time, we will run our lint script and get nothing:

Lint results - Nothing happened

Rules defined by others’s opinions

If you’re not really sure where to get started or just want to quickly get up and running, it’s recommend that you enable ESLint’s own recommended rules. Let’s add this to our .eslintrc.js config file:

module.exports = {
    "settings": {
        "react": {
            "version": "detect"
        }
    },
    "env": {
        "browser": true,
        "node": true,
        "es6": true
    },
    "plugins": [
        "react"
    ],
    "extends": [
        "eslint:recommended"
    ],
    "parser": "babel-eslint"
};

And let’s run our yarn lint.

Woah! This will immediately give you a lot errors, it seems like something’s wrong.

Lint results - React errors

Since we’re running a React app, we also want to make sure our linter understands the rules it should follow, so let’s also add our React plugin to the .eslintrc.js config setup:

    "extends": [
        "eslint:recommended",
        "plugin:react/recommended"
    ],

Now if you run yarn lint, you get something a little more logical.

Lint results - Normal errors

If you’re following along, it looks like our starter app had a misplaced semicolon and a missing prop in our propTypes validation for Layout.js. Writing this helped me fix my own repo! ?

Going further, if those don’t seem to fit your needs, lots of developers and teams have published their own configurations that ESLint allows you to easily tap into.

Some popular ones include:

• Airbnb’s config
• Semistandard
• Google’s JS Style Guide

Not happy with the options available? You can even create and publish your own to either layer on top of others as a starting point or give it a go from scratch.

Let it do the work (most of it)

You don’t think I’m going to make you fix all of those thing yourself do you? Well, you may have to fix some, but let’s try to get ESLint to fix some of it for us.

If you noticed after we ran the command above, ESLint gave us an extra message:

Lint results - Option to fix

So let’s give it a try! Let’s run:

yarn lint --fix

And what do you know, it now only gives us 1 linting error.

Lint results - 1 error

Turns out ESLint was able to fix our semicolon issue automatically, but we’ll still have to add pageName to our Layout’s propTypes manually, not too much of a lift.

Running one more time and finally nothing again! But this time because everything's looking good.

Lint results - No errors

Go forth and write messy code!

Bruce Almighty - Typing

Kidding ? The good news here, is now you can easily take a look at the general health of your codebase as well as hopefully fix most of it automatically. This is going to save a lot of headaches as you work with others on your team, and generally, it’s nice to have all of your code neat and tidy.

This post is just getting started. ESLint is a wide open book with tons of plugins and rules, and it’s not the only linting tool in the game. Play around and find what fits best for you and your team. The little time spent configuring it to your app will save you lots more time in the long run.

Other linting tools to check out

• JSHint: an alternative to ESLint
• Stylelint: a linting tool for CSS and CSS-like syntaxes like Sass
• Awesome ESLint: a simple list of awesome configs, parsers, plugins, and other tools to boost your ESLint game
• Webhint: linting tool for accessibility, speed, and more website best practices
• A11y JSX Plugin: ESLint plugin for checking accessibility rules on JSX elements

Originally published at https://www.colbyfayock.com/2019/10/what-is-linting-and-how-can-it-save-you-time

What are the code review best practices companies such as Microsoft follow to ensure great code review feedback? How do you stay productive while doing code reviews? Learn proven code review best practices from Microsoft in this article.

The benefits of code reviews rise and fall with the value of the code review feedback. If done correctly, code reviews can help to ensure a high-quality code base. However, if teams are not aware of and do not follow code review best practices, developers may experience several code review pitfalls. In the worst case, reviewing code can slow your team down.

I have been researching and working with teams at Microsoft for several years. Through several large scale-studies, we discovered a number of code review best practices that help teams stay productive and boost their code review value. But first, let’s start at the beginning. What does code review look like?

A typical code review process

A typical tool-based code review process starts when the engineer prepares the code for review. Then, she selects relevant reviewers for the code change. The reviewers are notified and give feedback on the code. The code review author works on the feedback until all parties are satisfied. Then, the code is checked into the common code base.

A typical tool-based code review

To ensure that this process is smooth and does not become a nightmare, it is important to understand code review pitfalls and which code review best practices you can follow to overcome those.

The main code review pitfalls are:

• not getting useful feedback,
• not having enough time to do code reviews,
• code reviews taking too long causing long waiting times.

The code review best practices I present below help counteract those pitfalls, by making the job of the reviewers as easy as possible. They also help the reviewer to focus on providing valuable feedback.

Code review best practices for code authors

In a code review, there are two different stakeholders: the code author who asks for feedback and the code reviewers, who look through the code change and provide the feedback. As a code review starts with the author, I explain the code review best practices for code authors first.

For my e-mail subscribers, I prepared an exclusive code review e-book including a checklist with all code review best practices. I also added additional bonus insights. You can request the Code Review e-Book here.

Read through the change carefully

The first code review best practice is to read carefully through the code change before submitting the code for review. There is nothing worse than asking several developers to look through the code and give feedback on issues you could have fixed yourself.

This wastes everyone’s time and it might make you look bad. For future code reviews, developers may also be reluctant to review your code.

So, ensure you use a code review tool or a diff tool that can highlight what changed between this and the previous version. Because the code is presented in a different way and changed code passages are highlighted, it makes it easier for you to review your code yourself before sending it out.

Often you will see changes that you actually forgot you made or missing issues highlighted you should fix before asking somebody to review.

The best time to fix issues is before the code is sent out for review. (Click to tweet).

_Thoroughly look through your code before submitting for review (Photo by Marten Newhall on Unsplash)_

Aim for small, incremental changes

As a developer, you should always strive for small, incremental and coherent changes. This best practice helps when working with code revision tools, such as git or SVN.

Small, incremental code changes are also a crucial code review best practice as other developers must be able to understand your code change in a small amount of time.

10 lines of code = 10 issues.

500 lines of code = “looks fine.”

Code reviews.

- I Am Devloper (@iamdevloper) November 5, 2013

If several changes with different purpose happen within one code review the task of code reviewing becomes more difficult. This also decreases the ability of code reviewers to spot problems with the code. In several studies, we see that the value of the code review feedback decreases with the size of the change under review.

On the other hand, you also want to make sure the changes are coherent. Rarely code changes are too small to be sent out. It happens, but, not that often.

The quality and value of code review feedback decrease with the size of the change. (Click to tweet).

Cluster related changes

Another code review best practice is to cluster related code changed. Imagine you plan to add some new functionality, fix a bug in another function, and refactor a class. Then, each of those changes should be a separate code review. This way, you ensure the purpose of the code change is clear to the reviewers. A clear purpose makes the reviewing job much easier and increases the feedback value.

Describe the purpose and motivation of the change

One way to make sure you invest your time right during code review preparation is to write a description of what this code change is all about. With a small note, you help the code reviewers to understand the purpose of the code change and also why you changed it. This code review best practice speeds up code review time, increases the quality and value of the feedback, and improves code review participation rates.

_Code reviewing isn’t a puzzle. Help reviewers focus on key issues by describing the code change. (Photo by Hans-Peter Gauster on Unsplash)_

Code reviewing isn’t a puzzle. Help reviewers focus on key issues by describing the code change. (Click to tweet).

Interestingly, in our studies, we observed that developers really appreciate code change description. They actually wish that more people would write descriptions. On the other hand, we saw that the same developers did not always include descriptions themselves.

One reason for this is that when you write the code yourself, you are so involved with the code that you think it is self-explanatory. Fact is, it is not.

And if you do not help the reviewers to understand the code, they will not be able to provide valuable feedback.

So, write the note, even if it just says: “Updated the API endpoint to be compliant with security regulations”.

How much easier did the job of reviewing the code just get with this note? Remember, code reviewing isn’t a puzzle!

Even if the code change seems trivial to you, add a description, so reviewers know what to expect (Click to tweet).

Run tests before submitting a code review

Yes, take the time to run the tests for your code change. Testing isn’t only a best engineering practice, but it’s also a code review best practice. Because testing your code ensures that the code actually works before you ask for feedback.

In addition, it shows that you respect the time of the code reviewers. It is not only embarrassing to send out code that obviously (as the tests show) is not working as expected, it also kills everyone’s productivity. So, run the tests first!

Automate what can be automated

As one of the main pitfalls for code reviews is taking too long, you better follow the code review practices of automating what can be automated.

Use style checkers, syntax checkers and other automated tools like static analysis tools to help improve the code. This way, you make sure that code reviewers can really concentrate on giving valuable feedback and do not need to use their time to comment on issues that can be found automatically.

Skip unnecessary reviews

You read that right. Some reviews can be skipped. Obviously, it depends on your organizational policies, but if they permit it, you might consider skipping code reviews.

But stop before heading out and telling your team you need no code reviews anymore. Skipping code reviews is only advisable for trivial changes that do not change the logic such as commenting, formatting issues, renaming of local variable or stylistic fixes.

Skipping unnecessary code reviews boosts your productivity. Click to tweet.

Do not select too many reviewers

You should select the right number of reviewers for your code change. If now numbers above 4 people come to your mind, I’d like you to stop right there. Because adding too many developers on code reviews does more harm than good.

One problem is that if you add too many developers, each one of them feels less responsible to give feedback. Another issue is that adding more people than necessary decreases your team’s productivity.

Some studies suggest the code review best practice of adding only two active reviewers.

For some code changes, you want additional experts like security experts or developers from other teams to look through the code. But, more often than not, two active reviewers are just fine.

Many code review tools allow notifying developers without making them mandatory reviewers. This ensures that they stay in the loop and are aware of what is happening, but removes the obligation for them to comment on your code.

Add experienced reviewers to get insightful feedback

Studies have shown that the most insightful feedback comes from reviewers that have worked on the code you are going to change before. They are the ones that give the most insightful feedback.

How often a reviewer has already reviewed code influences the ability to give useful feedback. Similar, experienced and senior developers tend to give better code review feedback.

But, be mindful about the workload of senior engineers, as they tend to be added as reviewers a lot.

Developers that changed or reviewed pieces of the code before, give the most valuable code review feedback. (Click to tweet).

Add junior developers to let them learn

One of the code review goals is training and learning, so do not forget to include junior developers. Consider adding reviewers that are not familiar with the code base, but that would benefit from the knowledge to allow knowledge dissemination.

Notify people that benefit from this review

For some people, like project managers or team leads, receiving notification about code reviews (without being actually required to do the code review) is beneficial. But, you have to take a conscious decision on whom you gonna notify. Not everybody really cares or should care about your code review.

Don’t notify too many people

Do not add everybody on the notification list. Only add people who actually benefit from the information that a code review is in the process.

I have seen teams, where each team member was added to each of the code review of the extended team by default (+70 people). This practice is like adding nobody to the list. Or in the worst case, you have several of your engineers spending their time looking through hundreds of code reviews to figure out if it’s relevant for them.

Give reviewers a heads-up before the review

A really effective code review best practice is to let your co-workers know ahead of time that they will receive a code review soon. This code review best practice reduces turn-around times substantially.

So, let them know a code review is coming their way as soon as possible.

Giving people a heads-up that a code review is on its way can speed up review time. (Click to tweet).

Be open to suggested changes

Receiving unexpected comments or feedback might make you tense and defensive. Try to prepare yourself mentally and work on your ability to be open to suggestions and different viewpoints. Always start with the assumption that the reviewer had the best intention.

If some feedback made you uncomfortable try to sort things out as soon as possible. Sometimes it is a good idea to have more personal face-to-face conversations to resolve some issues.

_Don’t be defensive if confronted with unexpected feedback. (Photo by Sweet Ice Cream Photography on Unsplash)_

Show respect and gratitude to the reviewers

Code reviews rise and fall with the team’s feedback culture. As a code author, show gratitude and value the received feedback. Make sure to carefully consider the reviewers’ feedback and communicate throughout the feedback cycle.

Tell the reviewers which actions you took and which decisions you made because of the received feedback in a respectful manner.

Code review rises and falls with the quality of the team’s feedback culture. (Click to tweet).

But, creating a great feedback culture is a two-way street. Naturally, code reviewers influence the culture a lot. So let us look closely at the code review best practices for code reviewers.

Code Review Best Practices for Code Reviewers

Being asked to give feedback on a code review is an honor, so you want to make sure you know how to give valuable code review feedback.

During code reviews, you can not only demonstrate your skills and knowledge but also mentor other developers and contribute to the team’s success. Nothing worth than investing time in code reviews and not getting valuable feedback.

Give respectful and constructive feedback

Even though it reads like a no-brainer, code reviews do put the code author in a vulnerable position, so you must be considerate of that.
Your job is it to give constructive and valuable feedback but also to do so in a respectful manner.

Especially using code review tooling, please reflect on how and what kind of feedback you give. It is just so easy to hurt someone feelings — especially in written form. Too often time pressure might make you give a sloppy answer that can be misinterpreted.

Go and talk in person if necessary

Code review tools and chat-tools allow us to communicate with our peers in an asynchronous and effortless way. But, there are quite a few situations where a proper human interaction, either face to face or via voice/video cannot be bet.

Complex issues, for example, can be much more efficient and positively resolved once you hop over to your colleague or call her and discuss it personally. The same holds true for contentious issues or sensitive matters.

Maybe it is a better strategy to write a private email or seek a personal discussion with the code author if you think you might hurt some feelings are make the engineer lose the face. So, whenever you face a complex issue or might hurt some feelings, rethink your communication channels and act accordingly.

Ensure traceability for decisions

Even though less traceable conversations, such as face to face or video calls can make a big difference for team dynamics, it is important to document the discussion. Especially the code review outcome should be tracked for future reference by using traceable tools such as the code review tool.

The code review tool is the right communication channel for all simple matters, as it allows the whole team to follow along, and enables to look-up decisions and understand code development after the fact.

_Leaving traces about your decisions and changes helps to understand code evolvement (Photo by Marten Bjork on Unsplash)_

Always explain why you rejected a change

Let’s be honest. Having a code change rejected isn’t something the code author will enjoy. So, it is important that you are thoughtful and explain your rejection in a polite, constructive and friendly way.

Explaining the reasons behind your decision does not only help the code author to learn and grow but also helps the author to understand your viewpoint. It also promotes an ongoing dialog with the author.

Tell the code author exactly what she has to do to get the change accepted.

If you have to reject a code change, explain exactly what has to happen that the change can be approved. (Click to tweet).

Code review best practices for boosting productivity

Some of the biggest challenges during code reviews, for both the code author and the code reviewer are time constraints.

As a reviewer, you might find it challenging to take time out of your daily tasks to review the code of your peers. But, code reviews can be very beneficial to you and the team if done in the right way.

Integrate code review into your daily routine

Structure your day-to-day business in a way that you set dedicated time aside just for doing code reviews. For example, plan to work on code reviews every day from 11 to 12 AM.

This way you make sure you can account the time for code reviews, and also make it an anticipated activity for you and your team. This schedule will come in handy every time you have a reflection on your work progress or an evaluation of your work.

Reduce task switching as it kills productivity

Switching from one task to another is costly. Knowing you do not stop whatever you do every time a code review comes along your way ensures you can work more focused.

Which time slots work depends on your workload, the number of code reviews you have to perform as well as on the time those reviews normally come in. In some settings, your team benefits from two (shorter) scheduled reviewing times, such as in the morning and before you leave the office. This way, your peers do not have to wait for your feedback too long.

_Task switching kills productivity (Photo by Tim Gouw on Unsplash)_

Task switching kills productivity. So have dedicated code review times. #codereview (Click to tweet)

Give feedback in a timely manner

It is not advisable to jump right into a code review, whenever the notifications pop up, because of context switching costs. Still, it has several advantages for you and the code author to review the code in a timely matter.

Giving feedback as soon as possible ensures that the code author is not blocked by waiting for feedback. Also, if the author has to wait too long, it becomes harder for her or him to remember the changes and incorporate the feedback. Remember long waiting times are a number one code review pitfall.

Being one of the first reviewers (especially if there are quite a few) also ensures your effort looking through the code actually adds value. If you are the fifth person inspecting the code, chances are you are not going to add new insights anymore. If that happens frequently, you should implement the code review best practice for selecting fewer reviewers.

Review frequently not in a big bang fashion

Research shows that you can give better quality feedback if you review frequently and therefore less changes at a time. That means that you do not wait until several code reviews pile up to look through them in one go. Instead, you stick to your schedule and review one code review (or even parts of one if it is a larger code review) at a time.

If code reviews are generally too large and take too long, you can suggest the code review best practices for small, incremental and coherent changes to the code review authors.

Give better quality feedback to code reviews by not letting them pile up. (Click to tweet).

Focus on core issues, less nit-picking

Your goal as a reviewer should be to help with core issues, such as bugs, architectural problems, structural problems or problems that will lead to maintainability issues.

Obviously, if you see typos, badly named variables or styling issues, you might also point that out. Still, this is not your main tasks and, understandably, over discussing minor issues isn’t valuable to code authors.

Use a review checklists

Another code review best practice is to use a systematic approach for code reviews. A code review checklist can speed-up and improve your code review performance. Instead of making one from scratch, download a ready-made list and customize it to fit your team’s practices and your needs. Be sure to look for a checklist that is tailored towards your technology stack.

Code review best practices checklist

Now you know all the code review best practices to make the most out of code reviews. If you enjoyed this post, consider subscribing to my email list.

I prepared an exclusive Code Review e-Book for my e-mail subscribers to help you remember the code review best practices. I also added other great insights and summaries about code reviews. Get the 12 page insights to code reviews now. Not a subscriber yet? Just sign-up.

Want more on Code Reviews?

Check out proven code review best practices, learn about which code review pitfalls you should avoid, and also how to boost your code review value with great feedback.

To stay in the loop and never miss a blog post, sign-up to my email list and get the exclusive code review e-book. You can request the Code Review e-Book here.

You find me on Twitter

Let's connect on Twitter to discuss software engineering topics and code reviews there.

Originally published at https://www.michaelagreiler.com on May 2, 2019.

When you are coding, interruptions really suck.

You are in the zone, flying high, killing it. And BAM… meeting, standup, insert interruption… Great!

In that context, code reviews can be perceived as another hurdle to productivity.

And frankly I can relate to that.

Code reviews are hard.

Not only do you need to stop what you are currently doing, you also need to immerse yourself into somebody else’s code. It takes a lot of energy just to switch your focus.

Code reviews are time consuming.

According to Slack Overflow’s 2019 survey, 56.4% of developers spend 4 hours or more per week performing code reviews. And it can represent up to 20% of a developer’s week!

Code reviews are frustrating.

As a submitter, it can be frustrating to get your pull request rejected, to wait hours if not days for a review. As a reviewer, code reviews can feel like an obstacle to a good productive day.

Yes, code reviews can sometimes be hard, time consuming and frustrating.

But they’re also a good way to share knowledge, prevent bugs, and reinforce your company’s culture among other things.

What follows is a manifesto for submitters and reviewers to bring back peace of mind into code reviews. ?

A Submitter’s Manifesto

As a submitter, here’s what you can do to increase your chances of getting your pull requests approved in a timely manner.

Submit when you’re done.

It sounds obvious, I know. But the thing is — most of the time if the machine doesn’t work, it’s not because it’s broken… it’s because it’s not plugged in!

Very small details can make a big difference on how your work is perceived. And you don’t want your colleagues to feel they are investing time and effort into reviewing work in progress code.

Me: “It was just a missing !”

You: “Yeah I know, but the whole thing didn’t work and it took me 20 minutes to spot it.”

So here’s what you can do:

• Self-test your code. Include WIP in title or label if you’re not done yet.
• Self-review your code. Use the diff report of your code editor or versioning tool to catch mistakes.
• Make sure the tests of your CI are green before assigning a reviewer, this will save them time.

_Don’t be this guy, obviously ? (G[iphy)](https://giphy.com/" rel="noopener" target="blank" title=")

Make smaller pull requests

I get it, it’s a big, important and complex feature and you might be tempted to submit a long pull request. Yet, most of the time, you are better off submitting smaller pull requests.

Code reviews take energy. Big code reviews even more. Don’t impose on your team a developer vs food challenge every time they review your code.

Be nice, cut it in smaller chunks. You are also doing yourself a favor:

• You’ll get more qualitative feedback. The longer a pull requests the fewer qualitative feedback per line of code you’ll receive. Keep your pull request small (not too small either) and you’ll increase your chances of getting great feedback on it.
• You’ll get them approved faster. It’s a win-win, by breaking down your work into smaller pull requests, you increase your chances of getting them approved faster.

LGTM ?

For the nerds out there, here’s a study conducted on a Cisco programming team. It shows that after 400 LoC the ability to find defects diminishes pretty dramatically. ?

The next principle helps with keeping pull requests size under control.

Narrow the scope

The scope of your pull request should be simple, unique and well-defined. That might be a feature, a user-story or a bug fix.

Making the world a better place, one LoC at a time ?

One way to think about it is that reviewers have a limited number of “attention credits” (like everybody). Every time they focus on something, they use 1 credit. What happens when they have 0 left?

LGTM ?

Do what you can to reduce the noise around your work. Be mindful of the reviewer’s attention span.

For instance, avoid void changes (like skipping lines). They don’t add any value and complicate the code review.

Similarly, if your pull request changes the behavior, don’t include changes to formatting. Conversely, if your pull request changes formatting, don’t include changes that affect the behavior. They might be overlooked by the reviewer.

Give context

Think about your pull request as documentation for new comers. Guide the reader with context.

Start with a self-explanatory title.

Good title taken from the xg2xg repo ?

Then, write a clear description to explain what you are doing and why are you doing it. What is the purpose of this pull request? Why is this change necessary? How did you approach this problem?

Good example of explaining why the change is necessary taken from react repo

The description is also is great place to point out unresolved issues and open questions. Reviewers might have suggestions to unblock you.

Are you working on a visible part of the product? Screenshots can help get your point across faster.

• Show before/after differences.
• Use colored arrows.
• Add screen recordings if you feel like it :)

From react repository

Finally, write information signs along the way to guide the reviewer through your reasoning.

Keep a clean commit history to make it easier for the reviewer to follow your step. Use comments to point out alternatives you explored.

Good comment example from Lodash

Welcome feedback ?

Rejection hurts.

Truth be told, code rejection hurts even more.

I’m seeing this a lot!

It’s alright. Don’t take it personally.

Comments and suggestions are an opportunity to learn and become a better software engineer ?

A Reviewer’s Manifesto

Congrats on making it this far! Now let’s look at a few principles that might help you become a better reviewer ?

Adopt the right mindset

There is no scenario where a team can benefit from a reviewer being mean or patronising. Be kind. Period.

_What are you trying to say my friend? ([Giphy](https://giphy.com/" rel="noopener" target="blank" title="))

Want to make code reviews more exciting?

Look for something you can learn from this review. A new library, a new method, a new concept, a simpler way to do things. What piece of knowledge will you extract from it?

If you are the more experienced developer, is there something you can share? How can you use this review to transfer knowledge to the submitter? How can you help them become a better software engineer?

Thanks for the tip mate! ?

How to actually do a code review

What to review

What am I even supposed to look for? Without clear guidance on what and how to review, it’s easy to get lost. Here is what you can do.

First off, check the purpose. Is this code accomplishing what it is meant to do? Are there parts of the new code that are not clear to you? Ask clarifying questions. The code is easily testable? Test it. There’s no need to go beyond if this square is not checked.

Ok now that the code works, time to focus on the implementation.

Think about how you would have approached this problem. Would you have done it differently? Is there potential for refactoring or abstraction? Is this re-inventing the wheel? Is this using standard code patterns?

What to not review

Because a piece of code has room for improvement, it doesn’t always mean it needs to be improved.

At the end of the day, code reviews are a tradeoff between quality and velocity and depending on the scope and stage of the project it might make sense to let a few things behind.

Similarly, you shouldn’t be doing things that can be automated. Let your favorite linter hunt for the missing semicolons and extra indentation. No need for an endless debate on tabs vs spaces.

Finally, don’t increase the scope of the pull request. If you think of new things that need to be done, create a new pull request /task for that matter.

Review in a timely manner

There are at least 3 good reasons to review pull requests in hours rather than days.

• The submitter can move to the next task quicker
• It reduces context switching cost
• It reduces the risk of merge conflicts between branches.

Opened 6 years ago. Be right back ?

Disclaimer: I just released GitRise, a tool that helps teams using GitHub & Slack review pull requests faster. I do think it can help with this one :)

How to give feedback in a code review?

When giving a feedback, the form matters as much as the substance.

Did you know that in written communication, neutral content looks more negative than it actually is? Beware of this bias and include emojis when needed to get the tone right in your comments.

Also, most of the time, even if you are pretty sure that there is a better way to do something, you are better off asking a question rather than requesting a change. Plus, questions sound less aggressive.

Example from ember.js repo

Finally, reward when things are done right. Code reviews are also a great place to give kudos to colleagues for doing a good job. Be creative and fun :)

? Congrats on reaching the end of this blog post!

? Thanks a lot for reading and let me know if you have any comments!

? I just released GitRise, a tool that creates pull requests reminders for teams using Slack & GitHub. Give it a try if you want. Looking forward to your feedback.

Code reviewing is an engineering practice used by many high performing teams. And even though this software practice has many advantages, teams doing code reviews also encounter quite a few code review pitfalls.

In this article, I explain the main code review pitfalls you should be aware of to ensure code reviewing does not slow your team down. Knowing which pitfalls and problems arise can help you to ensure a productive and effective code review experience. Those findings are based on a survey we conducted at Microsoft with over 900 participants.

A typical code review process

A typical tool-based code review process looks roughly like this: Once the developer has finished a piece of code, they prepare the code for being submitted for review. Then, they select reviewers who are notified about the review. The reviewers then review the code and give comments. The author of the code works on those comments and improves and changes the code accordingly. Once everybody is satisfied, or an agreement is reached, the code can be checked into the code base.

In another post, I described how a typical code review process looks like at Microsoft.

Code reviewing isn’t always a smooth process

These steps read like a smooth process. But, like everything, in practice, things tend to be more complicated than anticipated. During the code review process there a quite a few pitfalls that can reduce the positive experience with code reviews for the whole team. If not done correctly, code reviewing can also take its tolls on the whole team’s productivity. So, let’s have a look at the difficulties and pitfalls of code reviews.

The two main types of code review pitfalls are about the time spent on code reviews, and the value code reviews provide.

Be aware of code review pitfalls. Otherwise, code reviews can slow your team down. Click To Tweet

Waiting for code review feedback is a pain

One of the main pitfalls code authors face is to receive feedback in a timely manner. Waiting for the comments to come in and not being able to work on the code in the meanwhile can be a huge problem. Even though developers can pick up other tasks to work on, if the code review takes too long, it impacts the developer’s productivity and also the developer’s satisfaction.

But, why does the code review feedback take so long?

Developers have to juggle several responsibilities

Well, code reviewing is not the only task the code reviewer has to perform. On the contrary, code reviewing — even though it can take a significant amount of time of a developer’s day-to-day work — is only one part of the responsibilities and tasks of a developer. So, it is very likely that the code reviewer is engaged in other activities and has to stop or finish those first before looking at the code review.

If the timing is not ideal, and especially if the code reviewer hasn’t anticipated this change coming along, chances are, it takes a while before they look at the review. Remote teams also have to be aware of time differences. Otherwise, code reviews might even take longer.

Developers face problems if code reviews are not counted as actual work

Time constraints are real, and they affect both, the code reviewer and the author of the code. Doing a proper code review takes time. If teams want developers to do code reviews but do not value or count the time developers spend on code reviews, this becomes a real problem.

_You have to value and plan for the time spent doing code reviews.
Photo by [Unsplash](https://unsplash.com/photos/vcPtHBqHnKk?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText" rel="noopener" target="_blank" title="">freestocks.org on <a href="https://unsplash.com/?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText" rel="noopener" target="blank" title=")

You can’t expect quality code reviews if you don’t value the time a developer spends on them. Click To Tweet

Not rewarding code reviewing efforts and performance

It does not help to claim to value code reviews if you do not reward the effort developers spend on this task. Many companies focus on rewarding developers for the amount of code they write or the features they develop. This decreases the motivation and the ability of developers to do a good job helping each other (which includes code reviewing). Code review effort and performance should be a cornerstone for performance evaluation or promotion decisions.

If you want your team to do code reviews well, reward them for their work. Click To Tweet

Social factors and team dynamics

But waiting on a code review had not always to do with the lack of time or missing reward system. Due to its social character, delayed reviews can be due to insecurities or team dynamics. Especially if the code review is overwhelming, or if the reviewer is new to the code, doing a code review can be overwhelming:

I’m expected to participate, but I’m not quite sure how. I’ll wait until someone else starts. — study participant

Large reviews are hard to review

Another significant code review pitfall is large reviews. Imagine you are the reviewer, and you just got this review. You think, well, I am quickly going to look at that, but once you open the review, you see this large code change. Several files have been changed, and all changes tangle throughout the code base. What’s your first reaction?

Probably: holy cow!

That’s right. That is exactly what we saw when analyzing thousands of code reviews. Not only does review time increase with the size of the code change, but also feedback quality decreases. Well, that’s probably understandable.

10 lines of code = 10 issues.

500 lines of code = “looks fine.”

Code reviews.

_— I Am Devloper (@iamdevloper) November 5, 2013_

Large code changes are just incredibly difficult to review. If, in addition, the code reviewer is not that familiar with the part of the code base the change took place in, reviewing can quickly become a nightmare.

Large code reviews are hard to review. The quality of the review decreases with the size of the change, thus limiting the value teams get out of from code reviews. Click To Tweet

Understanding code changes needs some guidance

Understanding code changes, and especially the motivation for a code change, is another code review pitfall many reviewers face. If there is no description explaining the purpose of the change, code reviewing becomes much harder. We saw in the study that if the code reviewer does not understand the code change, or if she is overwhelmed by the amount of change, she cannot give insightful feedback.

It’s just this big incomprehensible mess. Then you can’t add any value because they are just going to explain it to you and you’re going to parrot back what they say.

— interviewed developer13

Not getting valuable feedback decreases the developers’ benefit from and motivation for code reviews

Without doubt, spending the time on code reviews and not getting useful feedback back is a problem. Even though the team might still benefit from the knowledge transfer, the developer’s motivation to do code reviews and the benefits from code reviews decrease when they do not get valuable feedback.

There are several reasons why reviewers do not or can’t give insightful feedback. It can be that the code reviewer did not have the right expertise. Another common reason is that the reviewer did not have enough time to look thoroughly through the change.

Maybe the code reviewer does not understand the code. It can also be that the code reviewer does not know what issues to look for. Understanding what makes for valuable code review feedback and implementing best practices mitigates this pitfall.

Once the main discussion is about styling, you need to act

Another problem that can happen during a code review is called bikeshedding. Bikeshedding means that developers focus on smaller issues and start disputing minor issues and overlook the serious ones.

The reasons for that are manifold. Common behind the scenes challenges that lead to bikeshedding is that developers do not understand the code change or that they do not have enough time for the code reviews. Sometimes bikeshedding can be a sign that there are issues with the team dynamics.

If people dispute about minor issues during code reviews, you have to take a look at the underlying issue. Time pressure, too large reviews, rivalry? Click To Tweet

Reaching consensus might need a face-to-face discussion

Sometimes it can happen that it is hard to reach a consensus. This can occur between code reviewer and code author, or also between several code reviewers directly. Such situations must be handled carefully as team dynamics are closely connected to these happenings. Communication via tools and in written form can aggravate this problem. If there seems to be any tension, or contentious issues to discuss, switching to face-to-face (either in person or via a video call) might be a good idea.

The benefits of code review outweigh the effort

I hope this list of code review pitfalls did not change your mind about code reviews. Because, the good news is that if you are aware of the code review pitfalls and counteract them, code reviews are a very beneficial engineering technique. And, there are even more proven ways to work effectively with code reviews.

Code review best practices

In the next blog post in this code review series, I show best practices to help to minimize the code review pitfalls and challenges and ensure your team gets the best out of the code review practice. So keep on reading. To be notified when I publish the next post, follow me on twitter.

I prepared an exclusive Code Review e-Book for my newsletter subscribers. So make sure you subscribe to my email list and secure your Code Review e-Book including a handy cheat sheet of code review best practices.

Originally published at https://www.michaelagreiler.com on April 6, 2019.

Have you ever wondered how one of the largest software companies world wide ensures high quality code through code reviewing?

So did I. That’s why together with my colleagues at Microsoft, we investigated how code reviews are done at our company. Is it a common practice? Are developers required to do code reviews? And which tools do they use?

Let’s find out in this post, which is part of a larger blog post series about code reviewing.

To begin with, let me give you some key information about Microsoft. Microsoft has around 140,000 employees. Approximately 44% of them, that means over 60,000 employees, are engineers. Several products such as Office, Visual Studio or Windows are developed by thousands of engineers that work on the same code base simultaneously.

I say all this to give you some context and perspective of what it means to coordinate and manage the software development process. As you can imagine, it is a non trivial task to ensure code developed by different sub teams actually works perfectly together. And code reviews play a big role at Microsoft to allow smooth collaboration at such a large scale.

Code reviews at Microsoft are an integral part of the development process

One of the important facts when it comes to code reviews at Microsoft is that it is a highly adopted engineering practice. Thousands of engineers perceive it as a great best practice. And most high-performing teams spend a lot of time doing code reviews.

At Microsoft, code reviewing is a highly adopted engineering practice and perceived as a great best practice. Click To Tweet.

Investigating code reviews at Microsoft

Because code reviews play such an important role in the Microsoft development process, it was an ideal target for us to dig deeper and really understand the benefits and drawbacks of this practice. In a large scale study on code reviews at Microsoft, we interviewed, observed and surveyed more than 900 developers about their code review practices.

Our aim was to understand how exactly code reviews are done at Microsoft, which challenges developers face while doing code reviews, and to distill which best practices they develop to overcome those challenges.

What can you learn from code review practices at Microsoft?

Most of the lessons learned are as valuable to smaller teams and organizations as they are for large teams and large organizations. In case your team does not do code reviews yet, I distilled our findings in a way that shows you the benefits of the practice. I also explain how the code review life cycle looks like so you can incorporate that practice in your own development process.

If your team already does code reviews, you can compare your practice with the code review practice at Microsoft. Does your code review lifecycle look different? In later posts, you learn from the challenges that arise doing code reviews and the best practices. With this information you can set out to see if your team already implements all the best practices I present and overcome challenges. But, let’s get started:

How often do Microsoft engineers perform code reviews?

In this study, 36% of the developers said they perform code reviews multiple times a day. Another 39% of the developers said they do code reviews at least once per day. 12% do code reviews multiple times a week, and only 13% said they did not do a code review in the past week.

This means that developers at Microsoft spend a significant amount of their time on code reviews. So it is important to make sure that this time spent is worthwhile. But, which benefits does code reviewing provide?

Which benefits does code reviewing provide?

The most important reasons developers mentioned as benefits of code reviews are to improve the code quality and to find defects in the code. Another important benefit of code reviews is knowledge transfer.

Knowledge transfer means that team members that review each others code, become familiar with a larger part of the code base. But, it also means that best practices are developed within the team. Another advantage is that new team members and junior developers can learn and improve their coding skills while reviewing or getting feedback.

If developers discuss alternative solutions during code reviews, it not only improves the code base, but has also a learning effect for all involved. Learning, mentoring and self-improvement are therefore all reasons code reviewing is perceived as such a beneficial practice at Microsoft.

Developers do code reviews to improve the code, to find defects, but mostly to increase the knowledge transfer amongst team members and for the learning effects. Click To Tweet.

But how does a developer typically do code reviews?

Code reviews can be performed in many ways. Sometimes, it is as informal as one developer walking over to another developer’s desk to look at some code together. Other times, teams review code together in groups. But the most likely scenario you will encounter for code reviews at Microsoft is that code reviews are done with the help of tools.

Code reviews at Microsoft are most often done via an internal tool

There is a wide variety of code review tools available, and at Microsoft teams are free to choose their tooling. In 2016, 89% of the developers indicate to use the CodeFlow code review tool. I will explain more about this code review tool at Microsoft later. Since then, and with the rise of Git, the tooling landscape has changed. I’ll add updated numbers as soon as they become available. But, let us consider a typical review situation:

Let’s imagine a developer at Microsoft, and let us call her Rose. Rose just finished a part of a feature and now wants feedback from her peers.

How does Rose start a code review at Microsoft?

Well, as said, Rose is ready to get some feedback. Therefore she first prepares the code for review. This step includes that she opens the code review tool, that allows her to preview the code change. The code review tool performs some diffing tasks that help Rose to see exactly which changes she has done.

After carefully reviewing those changes, she prepares a small note that tells the reviewers what she did and why she did that. This note helps the reviewers to understand the purpose of the code change and its motivation. Now the code is ready to be sent to the reviewers.

How does Rose select the right code reviewers?

Many experienced developers know who should be on the code review. Nevertheless, for people new on the team, or for new areas of work the selection can be a bit more tricky. If Rose does not know whom she should add, she would either look at the team policies or ask her colleagues. She can also use a recommendation feature of the code review tool that helps select reviewers based on experience and knowledge with the code base.

Who are relevant reviewers?

Rose selects reviewers that she thinks can contribute their knowledge to this piece of code. The reviewers are often other developers, but can also include other stakeholders, such as dev-ops engineers, UI experts, or also managers. Some reviewers are selected for their expertise, others are selected in order to stay informed about a coming change.

Rose requests feedback from her peers

Once everybody is selected, Rose sends out the code review (by pressing the send button ?). The code review tool sends notification automatically to inform everybody that a code review has been created. Notifications are sent to all reviewers. But, often additional parties, such as managers or product managers of other teams, are also added to the notification list, and are automatically informed for each review. Those notifications allow them to stay in the loop. They are not required to perform the review.

Receiving feedback is an iterative process

Once Rose’s colleagues have time, they will look at the code review. Each reviewer can annotate the code and add comments. Once finished commenting, the reviewer sends the annotated code back to Rose. Rose can now work on the comments and prepare a new improved version of the code.

Reviewers normally look for things like: does the code look bug free? Is there an architectural problem? Are there minor issues such as missing comments, spelling mistakes? Not all comments are equally valuable. But, there are several best practices to boost the value of code review comments.

Rose prepares a new improved version of the code

Rose works on the feedback by fixing and addressing the suggestions. If Rose sees that there are some misconceptions or other contentious issues, she might walk over to a colleague to discuss this in person. That’s sometimes easier and more personal than through the tooling.

Anyway, once she finished working on all the feedback, she sends a new version of the code to the reviewers. That new improved version is called a revision.

If needed, she will receive further feedback. Whether or not this cycle continues for a few times depends on the type of change and its quality. For simple and small code changes, often only one code review revision is needed. For other more complex changes or changes in problematic code, several iterations might be necessary.

It is totally normal, and partly desirable, that this code review feedback cycle sparks some discussions between the author and the code reviewers.

All reviewers approve and Rose checks-in the code

After this review cycle, reviewers mark the code as okay, and Rose can finally check-in the code into the common code base.

Some teams have policies that allow the developer to check-in the code before an actual review is completed. This is normally restricted to small and trivial changes, in order to allow asynchronous reviews and to speed-up development.

All steps I describe are part of a typical code review life cycle at Microsoft and are performed by all teams. Depending on the team’s policies, teams are more strict or rigorous for each of the steps.

Not all teams are the same

As you can imagine, not all 60,000 engineers, and not all of the thousands of teams do the same. Some teams at Microsoft have additional steps or tools they require during the code review life cycle. I want to give you a short overview of some additional steps that teams add to the code review process.

Code reviews including test results

The least what you want is to waste time by reviewing “automatic detectable” buggy code. I mean, if you could run automated tests and realize that the code does not work as expected, then, that’s what you should do: Run the tests before the review.

That’s why some teams require test results to be submitted with each code review. This way nobody can forget about running the tests. And it assures that the tests have actually run and passed for the given code change.

Other teams went even a step further and configured the code review tool in such a way that for each code review a developer submits, a build is triggered. That build contains that exact change, and also starts a series of automated tests. The results of this build and these tests are attached to the code review. Configuring it this way, ensures that the code changes have been tested with the latest code changes from the common code base.

Code reviews including user interface

If changes affect the user interface, it is also a smart idea to require the developer to submit a screenshot. That way the code reviewer can see the effects of the code change without running the code. Second, the code reviewer can spot discrepancies when running the code on her own machine.

Code reviews including static analysis

Static analysis tools are only as good as their configuration, but, in terms of styling issues they can save a lot of time for code reviewers. Some teams at Microsoft use automated static and dynamic analysis tools as dedicated bot-reviewers. Those bots comment on code styling and other static issues. Thus, freeing up time for human code reviewers to perform more interesting tasks.

Microsoft’s code review tool

For many years, one of the de facto standards for code review at Microsoft was an internal tool called CodeFlow. This is a sophisticated code review tool that supports developers and guides them through all steps of a code review. CodeFlow helps during preparation of the code, automatically notifies reviewers, and has a rich commenting and discussion functionality.

CodeFlow is an UI heavy tooling, much like Word or PowerPoint, as you can see in the screenshot below.

CodeFlow’s interface explained

You can skip this if you want, but for all interested, I am walking you through the interface of CodeFlow. Looking at the screenshot, on the left (A) you see all affected documents.

Also on the left, you see (B) the list of reviewers assigned to the review as well as their status (e.g., signed-off or pending). The active document is shown in the editor (C ). At the bottom, you see (D) a list of comments for all documents.

On the other hand, in the active document (F) is one single comment. This comment is connected to the concrete part of the code (i.e., one word in a line). Finally, at the top you see the overall status of the code review. In this case, completed. The numbers before signal the different revisions. In this review, there have been five revisions.

Commenting functionality

One of the nicest features of CodeFlow is its commenting functionality.

A code reviewer can select very precisely the parts of the code she wants to comment on. For example, a reviewer can even highlight just one or two characters in a line, instead of highlighting a whole row. Then, the reviewer can attach a comment to that selection.

The code author or other reviewers are notified of this comment and can start a conversation in form of a thread around this comment.

Discussion functionality

This commenting functionality feels like commenting on social media platforms, such as Twitter or Facebook. Therefore, the commenting experience in CodeFlow feels very natural and allows rich conversations and discussions. Another nice perk is the possibility to assign a status to each of these comment threads. The status can, for example, be “won’t fix”, “resolved” or “open”.

Comparison between code review revisions

A helpful feature is the possibility to select two different revisions of the code review and compare the differences between that. This means that you can see exactly which changes the code review author has performed between one code review revision and another one. That’s super handy to track the progress of the review.

Code review analytics tool

Developers spend a substantial amount of their time performing code reviews at Microsoft. To ensure this time is well spent, Microsoft has its own code review analytics platform.

This platform stores all code review data starting from the code under review, the developers involved in code reviews, to all comments of the developers. Even the code changes for each of the revisions can be traced back.

This code review data is the base for several empirical studies on code reviews at Microsoft. It is also used by many product teams for tracking their productivity and to understand their own code review practices. Also, many of the insights I share in this blog post series about code reviews at Microsoft stem from studies and analyses that involved this code review data.

The future of code review at Microsoft

With Micorosft’s engagement and acquisition of GitHub change was inevitable. The change is visible by the vast adoption of Git as a source control tooling within Microsoft for example. But, this also means that at Microsoft code reviewing in form of pull requests is on the rise.

I’ll definitely plan to address code reviewing using pull requests at a later time.

Coming up next: Code review challenges

I will write about code review challenges in the next blog post. To stay in the loop and follow me on medium.

Credit where credit is due:

I would like to mention my wonderful colleagues at Microsoft and the University of Victoria that have been part of this study: Chris Bird, Jacek Czerwonka and Laura Macleod and Margaret-Anne Storey. I loved to work with you on this ♥

Originally published at www.michaelagreiler.com on March 27, 2019.