Skip to content

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Dec 22, 2025

Fixes several tests. I'm not sure if we should put it behind a compat flag or not.

@anonrig anonrig requested review from a team as code owners December 22, 2025 21:02
@anonrig anonrig force-pushed the yagiz/throw-appropriate-errors branch from 94b64f3 to 4aec1ad Compare December 22, 2025 21:03
@anonrig anonrig requested a review from jasnell December 22, 2025 21:06
Copy link
Collaborator

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM but please don't land without coordinating around the landing of #5670

@anonrig anonrig force-pushed the yagiz/throw-appropriate-errors branch from 4aec1ad to 5b09510 Compare December 23, 2025 16:32
@codspeed-hq

This comment was marked as outdated.

@anonrig anonrig force-pushed the yagiz/throw-appropriate-errors branch from 5b09510 to 365ff3b Compare December 24, 2025 15:11
@anonrig
Copy link
Member Author

anonrig commented Dec 24, 2025

LGTM but please don't land without coordinating around the landing of #5670

Since that pull-request is divided into multiple pull-requests now, how do you recommend we proceed @jasnell?

@jasnell
Copy link
Collaborator

jasnell commented Dec 24, 2025

Just confirmed that this won't have any conflicts. Ok to land.

@anonrig anonrig merged commit 8c10cc1 into main Dec 24, 2025
21 of 22 checks passed
@anonrig anonrig deleted the yagiz/throw-appropriate-errors branch December 24, 2025 16:06
@jasnell
Copy link
Collaborator

jasnell commented Dec 24, 2025

The internal CI run on this appears to have failed with relevant errors. Were those investigated before this was merged?

image

@anonrig
Copy link
Member Author

anonrig commented Dec 24, 2025

The internal CI run on this appears to have failed with relevant errors. Were those investigated before this was merged?

All pipelines have passed on this pull-request. I believe you're looking at an old pipeline. https://gitlab.cfdata.org/cloudflare/ew/edgeworker/-/pipelines/1648206

@jasnell
Copy link
Collaborator

jasnell commented Dec 24, 2025

Odd. Ok, just going off what github ui is reporting.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants