Skip to content
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

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Nov 13, 2024

Summary

Align uv's workspace discovery with red knots (see astral-sh/ruff#14308 (comment))

  • Detect nested workspace inside the current workspace rather than testing if the current workspace is a member of any outer workspace.
  • Detect packages with identical names.

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.

@charliermarsh
Copy link
Member

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.

@charliermarsh
Copy link
Member

I can do the testing etc. if you need.

@@ -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 }}`
Copy link
Member Author

Choose a reason for hiding this comment

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

huh, why?

@MichaReiser
Copy link
Member Author

MichaReiser commented Nov 13, 2024

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)

@MichaReiser MichaReiser force-pushed the micha/workspace-discovery-nested branch from a767106 to 67b88aa Compare November 14, 2024 08:52
@MichaReiser MichaReiser marked this pull request as ready for review November 14, 2024 08:52
Copy link
Member

@charliermarsh charliermarsh left a 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!

@charliermarsh charliermarsh added the bug Something isn't working label Nov 15, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) November 15, 2024 03:51
@charliermarsh charliermarsh merged commit 7b4197b into main Nov 15, 2024
64 checks passed
@charliermarsh charliermarsh deleted the micha/workspace-discovery-nested branch November 15, 2024 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants