-
Notifications
You must be signed in to change notification settings - Fork 773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect nested workspace inside the current workspace and members with identical names #9094
Conversation
Thanks king! I should've clarified that I was also willing to do this if we shipped the version you have in red-knot, but I'm grateful to you for PRing it. |
I can do the testing etc. if you need. |
.github/workflows/ci.yml
Outdated
@@ -23,7 +23,7 @@ jobs: | |||
runs-on: ubuntu-latest | |||
outputs: | |||
# Flag that is raised when any code is changed | |||
code: ${{ steps.changed.outputs.code_any_changed }} | |||
code: ${{ steps.changed.outputs.code_any_changed }}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh, why?
I can do it tomorrow but let me know if you have any suggestions on how/where to add automated tests for this (considering that no tests are failing, it seems there are no existing tests) |
a767106
to
67b88aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, thanks Micha!
Summary
Align uv's workspace discovery with red knots (see astral-sh/ruff#14308 (comment))
Test Plan
I added two integration tests. I also back ported the tests to main to verify that both these invalid workspaces aren't catched by uv today. That makes this, technically, a breaking change but I would consider the change a bug fix.