Samuel Ruiz

Posted on Jan 20

What to Review Before Merging to `main`: A Practical Code Review Checklist

#webdev #javascript #react #programming

Merging to main is not a formality. It’s a commitment.
Once code lands there, it becomes someone else’s problem, future you’s problem, and potentially production’s problem.

Here’s a checklist I use before approving any pull request. If these points aren’t met, the code is not ready.

1. Unit Tests Exist (or There’s a Real Reason They Don’t)

If there are no unit tests, the default assumption is that the code is incomplete.

Questions to ask:

Are unit tests present for new logic?
If not, is there a clear, documented reason why?
Is this logic testable at all?

“No time” is not a valid reason.
“No idea how to test it” is a red flag.

2. Unit Tests Actually Assert Behavior

Tests without assertions are meaningless.

Things to verify:

Every test has at least one assertion
Assertions validate behavior, not just execution
Tests fail when the feature breaks

Bad tests give false confidence.
A green test suite that proves nothing is worse than no tests at all.

3. UI Matches the Design System

Code that “works” but ignores design standards creates silent debt.

Check for:

Correct colors (tokens, not hardcoded values)
Consistent padding, spacing, and alignment
Proper positioning across breakpoints
No “close enough” UI decisions

If the team has a design system, use it or explain why you didn’t.

4. The Code Is Readable, Simple, and Intentional

Before merging, ask yourself:

Does this code make sense at first glance?
Are variable and function names clear?
Is there unused or dead logic?
Is complexity justified—or accidental?

Also verify:

The code follows existing team patterns
No custom abstractions where simple logic would do
No clever tricks that reduce readability

Readable code scales. Clever code breaks teams.

5. It Works on Mobile and Touch Screens

Desktop-only thinking ships broken products.

Validate:

Responsive layout (small, medium, large screens)
Touch targets are large enough
No hover-only interactions
Scroll, focus, and keyboard behavior make sense

If it fails on a phone, it fails—period.

6. Folder Structure and File Placement Are Correct

Where code lives matters.

Check that:

Files follow the established folder structure
No random dumping into utils, helpers, or components
The structure communicates intent

Poor structure compounds fast and is painful to undo later.

7. Storybook Exists (When It Should) and Covers All States

If the project uses Storybook, it’s not optional.

Verify:

Stories exist for new components
All important states are covered:
- Loading
- Empty
- Error
- Disabled
- Edge cases

If a component has logic and UI states, it should be explorable in isolation.

8. Performance Has Been Considered

Performance issues rarely announce themselves—they sneak in.

Look for:

Unnecessary re-renders
Expensive calculations without memoization
Effects firing too often
Large lists without virtualization
Over-fetching data

You don’t need to over-optimize—but ignoring performance entirely is irresponsible.

9. ESLint and Prettier Are Clean

This is the easiest check and the least negotiable.

Before merging:

ESLint errors are fixed
Warnings are intentional and justified
Prettier has been run
TS compiler passes with no errors
No formatting noise in the diff

If tools can fix it automatically, humans shouldn’t review it.

Final Rule Before Merging

Before approving, ask yourself one last question:

Would I be comfortable owning this code six months from now?

If the answer is no, don’t merge it.

Code reviews aren’t about blocking people.
They’re about protecting the codebase—and the team—from unnecessary pain.

Here's a checklist you can follow

🧪 Testing

[ ] Unit tests exist for all new or changed logic
[ ] All unit tests include meaningful assertions (no empty or “render-only” tests)
[ ] Tests validate behavior, not implementation details
[ ] Edge cases are covered (errors, empty states, boundaries)
[ ] Tests are deterministic (no random values, real dates, or timing issues)

🎨 UI / UX (If Applicable)

[ ] UI follows the design system (colors, spacing, typography, positioning)
[ ] No hardcoded styles where design tokens exist
[ ] Layout is responsive (mobile, tablet, desktop)
[ ] Works correctly on touch devices (no hover-only interactions)
[ ] Accessibility basics are respected (focus, contrast, click targets)

🧠 Code Quality

[ ] Code is easy to read and understand
[ ] No unused variables, functions, or dead logic
[ ] Complex logic is justified and documented (or refactored)
[ ] Naming clearly reflects intent
[ ] Code follows existing team patterns and conventions

📁 Project Structure

[ ] Files follow the established folder structure
[ ] No “dumping” code into generic folders (utils, helpers, etc.)
[ ] New folders or patterns are explained and intentional

📘 Storybook (If Applicable)

[ ] Storybook stories exist for new components
[ ] All relevant states are covered:
- [ ] Default
- [ ] Loading
- [ ] Empty
- [ ] Error
- [ ] Disabled / Edge cases

⚡ Performance

[ ] No unnecessary re-renders
[ ] Expensive logic is memoized or optimized
[ ] Effects and subscriptions are scoped correctly
[ ] Large lists or heavy UI are handled efficiently

🧹 Tooling & Standards

[ ] ESLint passes with no errors
[ ] Prettier has been run
[ ] No unnecessary lint disables
[ ] CI checks are green

🧾 Final Confirmation

[ ] I would be comfortable maintaining this code in 6 months
[ ] This PR is ready to be merged into main

DEV Community