Automate the arguments away
Formatting and style are not worth human attention. A formatter applied automatically ends the discussion permanently and frees code review for the things that actually matter.
Any review comment that a tool could have made is a review comment that should have been made by a tool.
The checks worth having
- Automatic formatting on commit or in the pipeline
- Static analysis at a level the codebase can actually pass
- Dependency vulnerability auditing on every build
- Tests, run automatically, blocking on failure
- Type coverage increasing over time rather than all at once
Static analysis catches real bugs
Type errors, impossible conditions, undefined properties and unreachable code are all found before running anything. On a legacy codebase the first run is usually alarming and usually correct.
- Start at a level the codebase passes
- Increase the level gradually, fixing as you go
- Never suppress a warning without a comment explaining why
- Run it in the pipeline, not just locally
Review for design, not style
| Worth a review comment | Not worth it |
|---|---|
| Is this the right approach? | Bracket placement |
| What happens when this fails? | Variable naming preference |
| Is this tested? | Line length |
| Will the next person understand it? | Quote style |
| Does it handle the edge case? | Import ordering |
Consistency beats correctness of style
There is no single right way to format PHP. There is enormous value in a codebase where everything looks the same, because it makes unfamiliar code readable.
Pick a standard, automate it, and stop discussing it.