Skip to content

fix: Remove confusing zoneTag from CachePurgeResult#6600

Merged
danlapid merged 1 commit into
mainfrom
atewari/remove-zoneTag-Purge
Apr 17, 2026
Merged

fix: Remove confusing zoneTag from CachePurgeResult#6600
danlapid merged 1 commit into
mainfrom
atewari/remove-zoneTag-Purge

Conversation

@tewaro

@tewaro tewaro commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@tewaro tewaro requested review from a team as code owners April 17, 2026 03:41
ask-bonk[bot]
ask-bonk Bot reviewed Apr 17, 2026
View reviewed changes

@ask-bonk ask-bonk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

This PR removes the zoneTag field from CachePurgeResult to clean up the API.

Issues (ranked by severity)

  1. Build breakage (P0): The kj::String zoneTag member is removed, but zoneTag is still listed in the JSG_STRUCT macro on the same line. JSG_STRUCT generates accessors for each listed field, so this will fail to compile. The macro must also be updated.

  2. TypeScript type snapshots need regeneration (minor): types/generated-snapshot/ still includes zoneTag in CachePurgeResult. Per repo conventions, run just generate-types after fixing the C++ change to update the snapshots. (Don't edit the generated files directly.)

Comment thread src/workerd/api/global-scope.h Outdated
@ask-bonk

ask-bonk Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Review posted successfully on PR #6600. Here's what I found:

Two issues identified:

  1. Build breakage (P0): The kj::String zoneTag field was removed from the struct, but zoneTag is still listed in the JSG_STRUCT(success, zoneTag, errors) macro on line 206. This will fail to compile because JSG_STRUCT generates code that accesses each listed field. I posted a suggestion to fix the macro to JSG_STRUCT(success, errors).

  2. TypeScript type snapshots (minor): The generated type snapshots in types/generated-snapshot/ still reference zoneTag. After fixing the C++ code, just generate-types should be run to regenerate those files.

github run

@tewaro tewaro force-pushed the atewari/remove-zoneTag-Purge branch from 9efa690 to 589a7b8 Compare April 17, 2026 03:43
@github-actions

github-actions Bot commented Apr 17, 2026 •
edited
Loading

Copy link
Copy Markdown

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@tewaro tewaro force-pushed the atewari/remove-zoneTag-Purge branch from 589a7b8 to c84972a Compare April 17, 2026 04:05
@tewaro tewaro requested a review from a team as a code owner April 17, 2026 04:05
@tewaro tewaro requested a review from emily-shen April 17, 2026 04:05
@tewaro tewaro force-pushed the atewari/remove-zoneTag-Purge branch from c84972a to 1e21892 Compare April 17, 2026 05:53
@danlapid danlapid merged commit 556d2f6 into main Apr 17, 2026
35 of 37 checks passed
@danlapid danlapid deleted the atewari/remove-zoneTag-Purge branch April 17, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ask-bonk ask-bonk[bot] ask-bonk[bot] left review comments

@danlapid danlapid danlapid approved these changes

@emily-shen emily-shen Awaiting requested review from emily-shen emily-shen is a code owner automatically assigned from cloudflare/wrangler

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tewaro @danlapid