feat(pypi): enable pipstar by default #3225
Conversation
|
Need to wait for the #3247 so that I can finish fixing the tests |
aignas
force-pushed
the
aignas.exp.bzlmod.enable.pipstar
branch
from
b697f55 to
790a508
Compare
aignas
changed the title
aignas
force-pushed
the
aignas.exp.bzlmod.enable.pipstar
branch
from
790a508 to
7b35e52
Compare
Extra fixes: - add more logging to the failure - better METADATA files in tests - fix tests
aignas
force-pushed
the
aignas.exp.bzlmod.enable.pipstar
branch
from
7b35e52 to
10692ce
Compare
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables pipstar by default for bzlmod builds, which is a significant step forward. The changes in internal_config_repo.bzl correctly implement this switch. The test updates, including modifications to hub_builder_tests.bzl and test data, align well with this new default, ensuring the test suite remains valid. The improved error logging in whl_metadata.bzl is also a nice addition for debuggability. I've found one potential issue regarding the use of dep_template in test dependencies which could cause problems in the future.
aignas
changed the title
|
OK, PTAL. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enables pipstar by default, which is a significant change. The implementation involves updating the default flag value, removing old configuration mechanisms (config.bzl), and introducing a new way to pass dependency information through a packages list. The test suite has been extensively updated to reflect this new default behavior. My review focuses on improving the clarity of the changelog entry and fixing a minor typo in an error message. The core logic changes appear sound and consistent with the PR's goal.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
I think I have finished fixing up the remaining issues in our test code
to enable
pipstarinbzlmodandWORKSPACE.This switches the test to only consider
pipstarbeing enabled.Fixes #2949