Skip to content

Remove blob input stream autogate#6236

Merged
jasnell merged 1 commit intomainfrom
jasnell/remove-blob-autogate
Mar 4, 2026
Merged

Remove blob input stream autogate#6236
jasnell merged 1 commit intomainfrom
jasnell/remove-blob-autogate

Conversation

@jasnell
Copy link
Collaborator

@jasnell jasnell commented Mar 4, 2026

Autogate has been fully deployed

@jasnell jasnell requested review from anonrig and harrishancock March 4, 2026 14:49
@jasnell jasnell requested review from a team as code owners March 4, 2026 14:49
@ask-bonk

This comment was marked as resolved.

@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 4, 2026

I'm Bonk, and I've done a quick review of your PR.

This PR removes the fully-deployed BLOB_USE_STREAMS_NEW_MEMORY_SOURCE autogate, deleting the old BlobInputStream class and making streams::newMemorySource the unconditional code path in Blob::stream().

One minor issue flagged (posted as a PR comment since the line isn't in the diff):

  1. Low severity — stale include: src/workerd/api/blob.c++ still includes <workerd/util/autogate.h> (line 10), which is no longer referenced after this change. Not blocking, just dead weight.

Everything else looks correct — the autogate enum and stringify map are removed in lockstep, no dangling references remain, and the blob.h forward declaration cleanup is complete.

github run

Autogate has been fully deployed
@jasnell jasnell force-pushed the jasnell/remove-blob-autogate branch from 2c699ca to 362839f Compare March 4, 2026 14:53
@jasnell
Copy link
Collaborator Author

jasnell commented Mar 4, 2026

/bigbonk roast this pr

@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 4, 2026

Alright, let me take a look at this PR and give it the roast it deserves.


The Roast of PR #3644: "Remove blob input stream autogate"

Oh wow, +2/-68. James rolled out of bed, deleted more code than most people write in a day, and called it a PR. Living the dream.

Let's break this down: the autogate BLOB_USE_STREAMS_NEW_MEMORY_SOURCE has been "fully deployed" — four words that do all the heavy lifting so the PR description doesn't have to. The body is one sentence. The commit is one commit. The review is... already done by a bot that found a bug you missed. Rough.

Speaking of which — Bonk already caught the stale #include <workerd/util/autogate.h> sitting there on line 10 of blob.c++ like a forgotten lunch in the office fridge. You removed every reference to autogates from this file but left the include. That's the code equivalent of moving out of an apartment and leaving one sock behind. Just enough to haunt the next person who opens that file.

The actual change itself? It's fine. It's great, even. You deleted an entire class (BlobInputStream) and replaced a conditional code path with the unconditional one. The autogate enum and string map are removed in lockstep. No dangling references. Clean surgery — except for that one stale include, which is now going to live rent-free in this file until someone else cleans it up or you push a follow-up commit that'll have a diff of exactly one line.

Verdict: 68 lines deleted, 2 lines added, 1 include forgotten, 0 effort on the PR description. Ship it.

github run

Copy link
Contributor

@dom96 dom96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for cleaning this up!

@jasnell jasnell merged commit 6a9a877 into main Mar 4, 2026
24 checks passed
@jasnell jasnell deleted the jasnell/remove-blob-autogate branch March 4, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants