Skip to content

process: retro on #1204 merging with only 2/47 review threads addressed #1286

Description

@sahrizvi

Summary

#1204 ("--dir silently read a populated warehouse store as empty") merged with only 2 of 47 review threads addressed, per the commit message on the follow-up recovery commit (e6d3817146, "recover #1204 review debt"). The remaining 45 threads included a real correctness bug — 39/50 driver e2e tests were silently self-skipping (later fixed to 36 pass / 14 skip) — and an incomplete P1 where absoluteFileUriPath() was written but never actually wired in, so behavior was unchanged on main despite the fix appearing to land.

All of this was caught and recovered before the v0.11.0 release candidate (see #1238), and the recovery commit explicitly named every thread it addressed and tracked the one item it still deferred (#1237, connection registry cross-tenant scoping). So this is not a live release blocker.

The process risk is independent of this specific PR: a 47-thread PR merging at 2/47 addressed is a review-discipline gap that could recur on any future PR of similar size, and might not always get caught by a diligent follow-up commit the way it was here.

Proposed follow-up

Worth a retro on what allowed a PR to merge with 45 open review threads, and whether CI/branch-protection should gate merge on review-thread resolution count for PRs above some size/thread threshold. This is a process question, not a code fix.

Provenance

Found during the v0.11.0 stable release's CTO persona review.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions