A pull request adds fourteen lines to a checkout handler. Every one of them sits inside if (newCheckout), the description mentions the flag, and the approval lands four minutes later. Nobody checked that newCheckout had been at 100% in production since August. The fourteen lines reached every customer on the next deploy, with no ramp and no dashboard open.
Stopping it needed the flag’s production state, which lives in the flag platform and never shows up in a pull request. Most of the risky checks in a flag PR are lookups like this, and a feature flag code review checklist is the obvious place to write them down.
Key Takeaways
- Flag PRs come in three kinds (adding a flag, building behind one, removing one) and each fails in its own way.
- The diff can’t tell you what a flag is set to in each environment, so someone has to go and look.
- Code added inside an existing flag check ships at whatever that flag serves today. At 100%, that means live on deploy.
- For a removal, check that the branch being kept is what production serves.
- The fallback written next to each read decides what users get when the platform is unreachable. Keep it identical at every call site.
1. What the diff shows, and what it hides
Most code review is reading. A flag breaks that habit, because the code holds every behavior a feature can have and the platform decides which one each user gets in each environment. It can also change its mind on a Tuesday afternoon without anyone committing anything.
Which lookup matters depends on what the PR does to the flag.
Everything here is about application code: the PR that reads, extends or deletes a flag check. If your targeting rules and fallthroughs live in git and change through Terraform, those diffs need a different eye, and the GitOps workflow for feature flags has that list.
2. Adding a flag: the creation checklist
The PR that introduces a flag fixes its key, its fallback and the context it’s read with. Changing any of those later means hunting down every call site. We’d bet most teams approve this PR on the first read anyway, since the code is small and switched off.
Does the key in the code match the key in the platform? Exactly, down to the underscores. The SDK has no way of knowing that release_checkout_v3 was meant to be release-checkout-v3. It returns the default you passed, and the only admission that anything is wrong is a FlagNotFound reason nobody reads unless they ask for it. A typo like that can serve the old behavior to everyone for months. Ask for a link to the flag in the PR description and copy the key from there.
Is the key written once? When the same string literal sits in five files, each copy can be mistyped on its own and each one has to be found when the flag comes out. Pulling it into a constant solves the typo half. We prefer a small named function around the read, since it solves both and pins the default and the context in one spot too, which matters for the next two checks. Rule 3 of the best-practices pillar argues for one toggle point per flag at more length.
Does the name follow your convention? Ideally CI enforces it already, with a regex like the one in our guide to naming conventions. If nothing enforces it, say so in the review. Renaming a live key means a migration, so this is the last cheap moment.
Is the default value the old behavior? Every SDK read takes a fallback:
const enabled = client.boolVariation('release_checkout_v3', { user_id: user.id }, false)That false is what the call site serves if the SDK can’t reach the platform, if the key doesn’t exist, and after the flag gets archived. For a release flag it should be whatever users got before the flag existed. Then grep the key and check that every read passes the same fallback. Two reads with two different defaults will put one user on both sides of the feature during an outage, which makes for a confusing support ticket.
Is the evaluation context stable? Rollouts hash an ID to decide who’s in. Pass a request ID, or anything regenerated per call, and the user lands in a different bucket on every evaluation. Leave the ID out entirely and Featureflip skips the rollout rule for that visitor, who falls through to the next rule or the default. Find out which attribute your rollouts bucket on (user_id unless someone changed it) and make sure the code passes that one.
Who reads it, the server or a browser? Client-side SDKs only receive flags marked client-side visible. Miss the checkbox and the browser falls back to the default without an error. Separately, if the flag decides whether a user is allowed to do something, move the check to the server. A user can edit anything their own browser evaluates, and that’s why authorization behind a client-side flag made our anti-patterns list.
How does it end? Temporary flags get an owner and an end date before they merge. A kill switch or a plan gate gets marked permanent instead, so cleanup tooling leaves it alone. This is easy now, while the author still remembers why the flag exists, and the lifecycle management guide covers what goes wrong when it waits.
Are both paths tested? Production is on the off path today and one toggle away from the on path. Every SDK ships a test client built from fixed flag values, so covering both takes two test cases and no mocking.
3. Building behind an existing flag: check the state first
Back to the opening PR. A feature already has a flag around it, someone extends it, and the new lines land inside the same if. No flag gets created, so nothing prompts anyone to open the dashboard.
Code behind an existing flag ships at whatever that flag serves today. If the flag is still at 0% everywhere, the new code stays dark, and that’s likely what the author pictured. A flag that production has at 100% puts it live on deploy. Partway through a rollout, it reaches a slice of users that was picked weeks ago for a different change, and it goes out with the rest of the release.
Look the flag up before approving, in every environment, because staging and production regularly disagree. Then ask the author which of these they meant:
- The new code belongs to the same release and should go out at the current percentage. Fine, if everyone agrees that’s the plan.
- The new code is a separate change that happens to live nearby. It needs its own flag and its own ramp.
- The flag finished its job months ago and is only still in the code because no one removed it. Then this PR is reusing a finished flag, which is how Knight Capital lost about $460 million in 45 minutes in 2012 (a reused flag bit, and one server out of eight still running the dead code behind it). Remove the old flag and give the new work a fresh one.
A change to the on branch alone still has to leave the off branch working, because off is the fallback. And a second flag read inside the first gives the reviewer four combinations to reason about. Fold them into one named decision, or make the dependency a prerequisite flag so the platform enforces the order.
4. Removing a flag: confirm which branch is live
A removal PR deletes one side of the conditional and keeps the other, and the diff looks tidy whichever side it keeps. It can’t tell you whether users are on the kept side today.
Most of the time they are, since a flag normally comes out because its rollout finished. The exception is a flag someone switched off in the spring and forgot. On a list of old flags it wears the same Stale badge as a finished rollout, and an author working down that list can keep the on branch out of habit. Approve it and a feature somebody turned off on purpose comes back for every user on the next deploy. Featureflip’s stale detection shows the reason a flag went stale. If the reason is “rolled back”, the off branch is the one to keep.
As this post goes up, our own repository has open removal PRs from the flag cleanup Action for flags like flag-ownership and scheduled-changes. A tool wrote those diffs, and a person still has to check what production serves before any of them merge.
Is every read gone? Search every repository and service that might evaluate the key, mobile apps and background workers included, plus anything reading it through a constant or a wrapper. Test fixtures and config files count too. Leave the key in one of those and the next person to grep it will assume the flag is still live.
Does the PR archive the flag? It shouldn’t. A merged PR can sit undeployed for days. Archive the flag while a running build still reads it and every read drops to its coded default, which for a rolled-out flag usually means off (our question page on archiving a flag still in your code has the timing). Archive once the removal is deployed everywhere.
Is anything else depending on it? If other flags list this one as a prerequisite, the platform will refuse the archive later and the cleanup stalls halfway, so check the dependents before the code goes.
Is the diff small enough to review? One flag per PR. A flag with deep roots can take two, and the cleanup playbook explains the split (make the winning path unconditional first, delete the dead branch second). Removal PRs opened by a tool get exactly the same review.
5. A checklist you can paste into your PR template
Copy whichever section applies into the pull request description. Items marked (lookup) can’t be ticked from the code alone.
### Adding a flag- [ ] Key matches the platform exactly (link to the flag: ___) (lookup)- [ ] Key is read through one constant or wrapper- [ ] Key follows the naming convention- [ ] Default at every call site is the current behavior, and all call sites agree- [ ] Context passes the stable ID the rollout buckets on- [ ] Client-read flags are marked client-side visible (lookup)- [ ] No authorization decision behind a client-side flag- [ ] Owner and end date set, or flag marked permanent (lookup)- [ ] Tests cover both the off path and the on path
### Building behind an existing flag- [ ] Flag state checked in every environment (lookup)- [ ] New code is meant to ship at that state, or it gets its own flag- [ ] Flag isn't a finished one being reused for new work- [ ] Off path still works- [ ] No new nested flag read (or it's folded into one decision)
### Removing a flag- [ ] Kept branch matches what every environment serves today (lookup)- [ ] Key removed from every repo, service, test fixture and config file- [ ] PR does not archive the flag. Archive after the deploy- [ ] No other flag lists this one as a prerequisite (lookup)- [ ] One flag in this PRSix of the nineteen boxes are lookups.
6. Where Featureflip fits
Each lookup means leaving the PR for another tool. We’ve tried to make that trip short.
The MCP server has a flag_status tool that returns a flag’s enabled state, serving strategy and prerequisites for every environment at once. A reviewer with an AI assistant in the editor can ask for it mid-review and never open the dashboard. In code, variationDetail() returns the reason alongside the value, so a mistyped key surfaces as FlagNotFound in a test or a log line.
Expiry dates, a default lifetime per project and the permanent marker come with every plan, Free included. Flag owners start on Pro. Stale detection runs hourly from real evaluation traffic and records the reason per environment, which is how a removal reviewer tells a finished rollout from an abandoned one. Featureflip also refuses to archive a flag it can still see being evaluated, which stops an early archive that got past review.
The cleanup Action opens one pull request per dead flag and follows keys held in constants. An opt-in sweep archives each flag later, once its removal has merged and the old code has stopped reading it.
The shorter version
- Sort the PR first. Adding, building behind, or removing?
- Adding: check the key against the platform, keep one toggle point, make the default the old behavior everywhere, pass a stable ID, set an owner and end date or mark it permanent, and test both paths.
- Building behind a flag: read the flag’s state in every environment before approving. The new code ships at that state.
- Removing: confirm the kept branch is what production serves, remove every read, and leave the archive until after the deploy.
- Put the checklist in your PR template and mark the boxes that need a lookup.
Frequently asked questions
What should a code reviewer check in a feature flag PR?
First work out which kind of change it is. A new flag needs its key checked against the platform, the same default at every call site (the current behavior), a stable ID in the evaluation context, and an owner and end date or the permanent marker. Code added behind an existing flag ships at that flag’s state, so look the state up in every environment. In a removal, the branch being kept has to be the one production serves today.
Why isn’t code behind a feature flag automatically safe to merge?
Because the flag’s state is set in the platform, outside the code. New code inside the check of a flag that’s already at 100% in production goes live for everyone on the next deploy. Code behind a flag is only dark if the flag is off in the environments it deploys to, and a reviewer has to look that up.
How do you review a feature flag removal PR?
Look up what the flag serves in each environment and check the PR keeps that branch. A flag that was rolled back looks the same in code as one that was rolled out, and keeping the wrong side can switch a feature back on months after someone turned it off. Then confirm every read of the key is gone across all services and tests, and that the PR doesn’t archive the flag. Archive after the removal is deployed.
What default value should a feature flag have in code?
The old behavior. The default is what the call site falls back to whenever the platform can’t give it an answer, archived flags included, so it should be the safe, known path. Every call site that reads the same key should use the same default, or one user can see both sides of the feature during an outage.
How do you catch a typo in a feature flag key?
Read the key through one constant or wrapper function, copy it from the platform rather than typing it, and ask for a link to the flag in the PR description. At runtime, an SDK reports an unknown key as FlagNotFound when you ask for the evaluation reason, so a test or a log line on variationDetail() will catch it. Without one of those, a mistyped key serves the default until somebody happens to notice the feature never turned on.
Featureflip is a flat-priced feature flag platform with expiry dates, permanent-flag markers, stale detection from real evaluation traffic, and a GitHub Action that opens one reviewable pull request per dead flag. Paid plans include unlimited flags, so giving new work its own flag never costs more than reusing a finished one. Start on the Free plan without a credit card.