Code Review

Code Review

Every PR to Apache Traffic Server goes through code review before merging. This page describes what reviewers look for, the approval requirements, and the merge process.

Review Process Overview

1. Submit PR — Author creates a PR against master (see Contributing)
2. CI runs — Automated checks execute (see Continuous Integration)
3. Review — One or more committers review the code
4. Address feedback — Author updates the PR based on review comments
5. Approval — At least one committer approves
6. Merge — A committer squash-merges the PR

What Reviewers Look For

Correctness

• Does the code do what the PR description says?
• Are edge cases handled?
• Are error paths handled properly?

Style and Conventions

• Does the code follow the Coding Style guide?
• Is the code formatted? (CI enforces this, but reviewers catch convention issues clang-format doesn't)
• Are naming conventions followed?

Testing

• Are there tests for the new functionality?
• Do existing tests still pass?
• For bug fixes, is there a test that would have caught the bug?

Performance

• Does the change introduce any blocking operations in event threads?
• Are allocations reasonable?
• Could this regress performance for common paths?

Security

• Are inputs validated?
• Are buffer boundaries checked?
• Does the change affect TLS, authentication, or access control?

Documentation

• For new features or configuration changes, are the Sphinx docs updated?
• For new config records, is the documentation complete?

Approval Requirements

From .asf.yaml:

• Minimum 1 approving review from a committer
• Stale reviews are dismissed when new commits are pushed
• All required CI checks must pass (see below)

CI Requirements

The following checks must pass before a PR can be merged (defined in .asf.yaml):

Check	What It Validates
AuTest 0of4, 1of4, 2of4, 3of4	End-to-end integration tests (4 parallel shards)
Rocky	Build on Rocky Linux (with ASAN + QUIC/Quiche)
CentOS	Build on CentOS
Clang-Analyzer	Static analysis
Format	clang-format compliance
Debian	Build on Debian (with hardening flags)
Docs	Sphinx documentation build
Fedora	Build on Fedora
RAT	Apache license header check (Release Audit Tool)
Ubuntu	Build on Ubuntu (clang, with hardening flags)
OSX	Build on macOS
FreeBSD	Build on FreeBSD

If a CI check is failing for reasons unrelated to your PR (infrastructure issue, flaky test), communicate in Slack (#traffic-server) or file an issue.

Merge Policy

Squash Merge Only

The project uses squash merge exclusively. Rebase and merge commits are disabled. This means all commits in your PR are combined into a single commit on master.

Write a clear, descriptive PR title — it becomes the commit message on master.

Who Can Merge

Any committer can merge any PR after review, with these requirements:

• All CI checks pass — never merge a PR with failing CI
• At least one approving review with no pending requested changes
• Milestone and labels are set appropriately
• PR targets master (only the Release Manager merges to release branches)

Committer Responsibilities at Merge Time

• Verify the PR title is a good commit message
• Ensure milestone and labels are set
• If there's an associated issue, verify it will be closed by the merge
• For backport PRs, only the Release Manager merges to the release branch

Requesting Reviewers

• Use GitHub's "Reviewers" sidebar to request specific people
• If you're unsure who should review, the community will self-select
• For plugin changes, try to get review from someone familiar with that plugin
• For core changes (iocore, proxy/http), broader review is valuable

Responding to Feedback

• Address all review comments — either make the change or explain why not
• Push new commits to the same branch (don't force-push during review — it makes it harder to see what changed)
• Re-request review after addressing feedback if the reviewer hasn't re-engaged
• Pushing new commits automatically dismisses previous approvals, so you'll need re-approval

Backport Merges

Backports to release branches (e.g., 9.2.x) follow additional rules:

• Only the Release Manager for that branch should merge backport PRs
• Backport PRs should have the Backport label
• The PR should target the release branch, not master
• The change should already be merged to master first (in most cases)

See Also

• Contributing — How to submit a PR
• Continuous Integration — CI checks that must pass
• Coding Style — Code standards reviewers enforce