-
Notifications
You must be signed in to change notification settings - Fork 582
Simple upgrades to Bzlmod migration tool #6251
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
Conversation
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.
Code Review
This pull request introduces a couple of useful improvements to the Bzlmod migration tool. The regex for parsing the workspace name is now more robust, and an important note has been added for users of python_register_multi_toolchains. I've suggested a small improvement to the formatting of the new message to ensure it's displayed correctly to the user. The removal of the add_python_repo function is also noted, presumably as dead code removal.
Require module maintainers' approval for newly pushed changes.
|
Fixed it with minor changes so it's more readable to users. With the current change invoking migration tool in the python extension test env looks: Before this change it was one more which is unsuitable next to the previous IMPORTANT note regarding python version. |
|
Please address the format issue. |
Require module maintainers' approval for newly pushed changes.
|
@meteorcloudy PTAL :) |
python.toolchain()call for each version inpython_register_multi_toolchains. This is hard to select from the migration tool itself due to being visible only for "python_3_x_toolchains" targets inresolved_deps(these targets are not used directly in most of the cases).