Most code review feedback is a request for changes, and the author edits the same file. Migrations break that rule: once a migration is merged and run anywhere - staging, a teammate’s machine, production - the file is a fact. Editing it changes nothing that already ran and silently forks the databases that have not. Reviewing migrations well matters more than reviewing almost any other diff, because it is the last time the change is cheap.
The review checklist#
Does down() actually reverse the up()? A down() that drops the table when up() added a column is a trap for the next rollback. If the change is genuinely irreversible - a dropped column - that should be a shouted comment, not a silent absence.
Are defaults and nullability safe for existing rows? Adding a NOT NULL column without a default to a table with a million rows either fails or rewrites the table while holding a lock. The safe shape is nullable first, backfill, constrain later - the same three-step dance as zero-downtime deploys require.
Did new query paths get indexes? A column added for a lookup (WHERE stripe_customer_id = ...) without an index is a slow query waiting for traffic. The migration is the cheapest place the index will ever be added.
Is data surgery mixed into schema changes? A migration that both alters a table and rewrites its rows couples “change the shape” with “change the content” in one irreversible step. Schema in migrations, data in a one-off artisan command you can rerun and monitor.
After merge: hands off#
The corollary rule, worth stating in the team’s written workflow: a merged migration is never edited. Found a typo in a column name after merge? A new migration renames it. This feels heavy the first time and is obviously correct the first time someone hits a database that ran the original.
Review migrations slowly, then let them harden into facts. That asymmetry - expensive to change, cheap to get right beforehand - is exactly what the habits in Code Review Habits That Actually Catch Bugs are for.
Share this post
Keep reading
Branch Protection Rules That Stick
Three rules cover most of the risk on main: review required, tests green, no force-push. More than that tends to get routed around.
Merging vs Rebasing on a Small Team
Rebase to tidy your own branch, merge to combine into main. One sentence of policy saves a lot of history arguments.
How to Review a Large Pull Request Without Losing a Day
Sometimes the big PR lands in your queue anyway. A reading order that surfaces the risk first, and how to give useful feedback instead of a rubber stamp or a rewrite request.
