Skip to content

Conversation

alexeagle
Copy link
Collaborator

No description provided.

Copy link

github-actions bot commented Sep 18, 2025

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

@alexeagle
Copy link
Collaborator Author

@fhanau I can't see why this one is red on internal - it's rebased today, can you give me hints?

@fhanau
Copy link
Contributor

fhanau commented Sep 24, 2025

@fhanau I can't see why this one is red on internal - it's rebased today, can you give me hints?

I believe it's failing because the internal build still uses WORKSPACE-based python, so it's tripping up on https://github.com/cloudflare/workerd/pull/5129/files#diff-c44211133c16ec2e8919d59a20a200b01720663667e0a2356e7db1ef2b3a15eb

@alexeagle
Copy link
Collaborator Author

It sounds like I'm blocked on making a matching internal change then? The old symbol is no longer available in rules_python after the switch. Can you make a corresponding change along with merging this?

@fhanau
Copy link
Contributor

fhanau commented Sep 24, 2025

It sounds like I'm blocked on making a matching internal change then? The old symbol is no longer available in rules_python after the switch. Can you make a corresponding change along with merging this?

I'll try – maybe there's a way to define an alias and avoid downstream changes, but doing the same kind of change won't be hard either.

@alexeagle alexeagle marked this pull request as ready for review September 25, 2025 03:28
@alexeagle alexeagle requested review from a team as code owners September 25, 2025 03:28
@fhanau
Copy link
Contributor

fhanau commented Sep 25, 2025

Made a downstream PR (temporarily blocked but should be ready tomorrow), internal build CI job is passing. Changes here LGTM, but I think we should drop the configure_coverage_tool variable here (we barely use python and don't need coverage), and remove the obsolete/commented out code in build/deps/python.MODULE.bazel.

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