The most valuable pull request that quarter deleted 4,000 lines

TL;DR#

A feature was switched off eighteen months ago by a flag that is now permanently false. Its code is still in the repository, and it is not free: it appears in every search, gets reviewed in every nearby change, gets migrated during every refactor, and makes the module look more complicated than it is.

We deleted 4,000 lines of it. Build time dropped, two in-progress migrations got smaller, and the part of the codebase everyone described as "the scary module" stopped being scary — because two-thirds of what made it scary was unreachable.

The work is not the deletion. It is proving the code is unreachable, and that is a genuine engineering task with a method.

The case this post is about#

A payments module with two implementations behind a flag:

// 2,100 lines
class LegacyCheckoutFlow(…) { … }

// 1,400 lines
class CheckoutFlowV2(…) { … }

fun checkoutFlow(flags: FlagSnapshot): CheckoutFlow =
    if (flags.isEnabled("checkout_v2")) CheckoutFlowV2(…) else LegacyCheckoutFlow(…)

Plus 500 lines of tests for the legacy path and a LegacyOrderAdapter keeping the old data shape alive.

checkout_v2 reached 100% rollout in March of the previous year. Nobody switched it back. Nobody deleted anything, because nobody could say with certainty that nothing used the old path.

The symptom#

Not an outage — a steady tax, in four forms.

Every nearby change touched it. A currency-formatting fix had to be applied twice, to both flows, with two sets of tests:

PR "fix rounding in order totals"
  files changed: 8   (4 of them in the legacy path)
  review comments: 11 (6 asking what the legacy path is for)

It was in the blast radius of two migrations. A Kotlin version bump and a dependency-injection rework both had to update the legacy code:

DI migration: 34 files to update, 19 of them in LegacyCheckoutFlow

It dominated search results. Searching chargeCard returned the legacy implementation first, so new engineers read the wrong code and asked about behaviour that had not shipped in a year.

And it distorted a design decision. During a planning discussion, the legacy flow's structure was cited as a constraint — "we can't do that, the checkout flows both need X" — about a flow that no longer ran.

That last one is the real cost and the hardest to notice.

Why the obvious fix failed#

"Just delete it." Blocked, correctly, in review: nobody could demonstrate that nothing called it. The flag looked permanently on, and "looked" is not an argument for deleting a payments code path.

"Leave it, it's not hurting anything." The four symptoms above are what it was doing. The cost is diffuse and continuous, which is exactly why it never gets prioritised against anything with a ticket.

"Mark it @Deprecated." We did, a year earlier. Deprecation annotations on code nobody calls produce no warnings, so it changed nothing and added to the impression that someone was handling it.

"Delete it next time we touch the module." The plan of record for eighteen months. "Next time" is always under time pressure, and deleting 4,000 lines is never the smaller change.

The actual mechanism#

Dead code has four costs, and only the first is the one people think of.

Cognitive cost. It appears in searches, in file trees, in review diffs. Every reader pays to determine it is irrelevant, and new readers pay most.

Maintenance cost. It compiles, so it must keep compiling. Every language upgrade, dependency bump, lint rule and framework migration includes it. This is the cost that scales with how much your ecosystem moves.

Blast-radius cost. It is in the set of things a refactor must update, so every structural change is larger and riskier than the live code justifies.

Decision cost. It appears to constrain designs. Nobody checks whether a cited constraint is reachable before accepting it.

And the reason it survives: deletion requires proof, and proof requires work. "I think nothing uses this" is not reviewable in a payments module. The absence of a cheap way to be certain is what makes the pile grow.

Deleting is cheap. Proving it is safe to delete is the engineering work, and it is worth budgeting as a task rather than hoping it fits inside another one.

And the sub-case that turns a good proof into a bad one: zero observations is a statement about your observation window, not about reachability. Two weeks of no hits proves nothing about a path that runs quarterly.

The shapes that bite, roughly in order of how often they have:

• Periodic work — a month-end reconciliation, an annual renewal sweep, a billing retry that only fires after three failures. The window has to exceed the period, and for the annual case that is not a window anyone will wait for.
• Seasonal or regional paths — a tax rule that activates in one quarter, a payment method used in one market during one holiday.
• Failure-only paths — the branch that runs when the primary provider is down. Zero hits means the provider stayed up, which is the outcome you wanted and the opposite of the conclusion.

The last one is the genuinely dangerous category, because the instrumentation is most convincing exactly when the fallback has been least exercised. Reachability by reflection, DI, or a config-driven class name has the same property from the other direction: static analysis finds no callers because there are no static callers.

So pair the counter with a reason the window is sufficient. "No hits in 14 days" is data. "No hits in 14 days, and this path has no periodic trigger, no failure trigger and no reflective entry" is a proof.

The fix#

Prove it with evidence, then delete in a sequence where each step is reversible.

Step 1 — prove it at runtime, not by reading. Instrument the dead path and ship it:

fun checkoutFlow(flags: FlagSnapshot): CheckoutFlow =
    if (flags.isEnabled("checkout_v2")) CheckoutFlowV2(…)
    else {
        log.atWarn()
            .addKeyValue("event", "legacy_checkout_entered")
            .addKeyValue("user_id", currentUserId())
            .log("legacy checkout path entered")
        metrics.counter("legacy_checkout_entered").inc()
        LegacyCheckoutFlow(…)
    }

Then wait. Not a week — a full seasonal cycle for anything with periodic behaviour:

legacy_checkout_entered, 90 days:  0

Zero over 90 days, with an alert configured to page if it ever became non-zero, is the evidence that makes the deletion reviewable.

Step 2 — check the non-obvious entry points. Runtime instrumentation covers code paths; it does not cover things that call in from outside:

$ gh search code 'LegacyCheckoutFlow' --owner ourorg     # other repos
$ grep -r 'LegacyOrderAdapter' analytics/ support-tooling/ ops/
$ # reflection and DI: a class referenced only by string name has no call site
$ grep -rn 'LegacyCheckoutFlow' --include='*.xml' --include='*.json' --include='*.yaml' .

The reflection check found one: a Koin module registering it by type for a screen that no longer existed. A pure call-graph analysis would have missed it.

Step 3 — remove the flag branch first, keeping the class. A small, obviously-safe change:

fun checkoutFlow(): CheckoutFlow = CheckoutFlowV2(…)

Now the class is genuinely unreferenced, and the compiler and lint can say so.

Step 4 — delete, in one commit, with the evidence in the message:

refactor(checkout): remove LegacyCheckoutFlow and its adapter

Unreachable since the checkout_v2 rollout completed 2025-03-14.

Evidence:
- legacy_checkout_entered counter: 0 over 90 days (2026-07-28 → 2026-10-26)
- org-wide code search: no references outside this module
- DI/reflection scan: one Koin registration, removed in #4821
- flag branch removed in #4830; class unreferenced since

Removes: LegacyCheckoutFlow (2,100), LegacyOrderAdapter (410), tests (1,490)

The commit message is the artifact that makes this recoverable later. git revert brings it back, and the rationale explains why it went.

And a guard so the next one does not accumulate:

# CI: a feature flag older than 90 days at 100% must be either removed or annotated
- run: ./scripts/stale-flag-audit.sh

❌ checkout_v2: 100% for 580 days, no removal ticket
   → add a removal ticket or annotate with an expiry rationale

How we proved it#

The runtime counter is the proof, and it is worth noting what makes it trustworthy: it was shipped to production and left for a full quarter, with an alert on non-zero. A 90-day zero with a page configured is a much stronger statement than a code read.

Then the measured effects of the deletion:

                            before    after
module lines                 9,840     5,840
module build time (clean)      47s       31s
test suite (module)           2m14s     1m22s
DI migration files to update      34        15

And the effect nobody expected:

"scary module" — files new engineers were told to avoid
  before:  7
  after:   2

Two of the five that stopped being scary were simply gone; the other three were legible once the duplicate implementation was not interleaved with them in search results.

One honest miss. Three weeks after the deletion, a quarterly reconciliation job failed — it had read the legacy_orders table that LegacyOrderAdapter wrote. The code was unreachable; the data was still consumed, by a job that runs four times a year and had not run during our 90-day window.

job: quarterly_revenue_recon
  FAILED  relation "legacy_orders" does not exist

Fixed in two hours by pointing it at the current table. The lesson is specific and worth keeping: a 90-day observation window does not cover a quarterly consumer, and runtime instrumentation of a code path says nothing about who reads the data it produced. For anything touching persisted data, check the consumers of the data separately from the callers of the code.

What this generalizes to#

Unreachable code accrues cost continuously and silently. Build time, migration scope, review load, search noise, and occasionally a design decision. None of it shows up as a ticket, which is why it survives indefinitely against work that does.

Proving unreachability is the task; budget it. Instrument, wait, search outside your repository, check reflection and configuration, then delete. Hoping it fits inside another change is how eighteen months pass.

Measure at runtime, not by reading. A call-graph argument misses reflection, DI by name, cron jobs, other repositories and external consumers. A counter in production and an alert on non-zero is cheap and conclusive in a way reading is not.

Match the observation window to the slowest consumer. Our 90 days missed a quarterly job. Whatever your longest-period consumer is — monthly billing, annual reports, a seasonal campaign — the window has to exceed it, and data consumers must be checked separately from code callers.

Related#

• The migration that could not be rolled back — the same expand-contract discipline for data
• A feature flag read on four platforms needs one source of truth and four caches — the flag that outlived its rollout
• The iOS framework you export is an API, and nobody reviews it — surfaces that grow by omission

Keep reading#

This is part of 100 practices of senior engineers — the habits that separate five years of experience from one year five times. Follow MobileByteSensei for the rest.