# Code Review Guidelines

## Purpose

Code review exists to catch bugs, share knowledge, and maintain code quality. It does not exist to relitigate architectural decisions that were made six months ago.

## Process

1. Open a pull request
2. Assign at least one reviewer
3. Reviewer reviews within 24 hours (48 hours if it's a large PR, 1 week if it's a PR from the intern)
4. Author addresses feedback
5. Reviewer approves
6. Author merges

## What to Look For

- Does the code work?
- Is it readable?
- Are there tests? (Aspirational at TomlTech, but we're trying)
- Are there obvious security issues?
- Does it follow the existing patterns in the codebase, even if those patterns are questionable?

## What Not to Do

- Do not rewrite the entire PR in your review comments
- Do not leave "nit" comments on code that is functionally correct and readable
- Do not request changes because you would have done it differently
- Do not approve without reading the code (we know you do this, please stop)
- Do not leave the comment "looks good to me" on a PR that deletes the authentication system

## Style

We use Rubocop with the default configuration plus 14 custom rules that nobody remembers adding. If Rubocop passes, don't argue about style. If you disagree with Rubocop, open a PR to change the Rubocop config. Nobody has ever done this because it requires understanding the Rubocop config, and the Rubocop config is 400 lines long.

## Turnaround Time

Reviews should be completed within 24 hours. If a review is not completed within 48 hours, the author may merge without review. This rule was added after a PR sat open for 3 weeks and the branch diverged so far from main that the merge conflict was larger than the original PR.

## The "Gary Rule"

Named after a former employee. If a PR introduces a gem dependency, the reviewer must verify that:
1. The gem is actively maintained
2. The gem has more than 3 downloads
3. The gem author is not named Gary