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

Fix a few preexisting issues noticed during review #1

Merged

Conversation

maresb
Copy link

@maresb maresb commented Mar 5, 2023

While reviewing your PR I got confused by some strange preexisting code, which I clean up here.

Filtering categories is opt-in, thus the default of True is wrong.
There is only one active usage of `make_lock_files` which sets
`filter_categories` explicitly, so changing this value shouldn't
break anything.
These lists are not optional, so `or []` doesn't seems to accomplish
anything.
This is obsolete since we no longer use aggregate_lock_specs to
aggregate specs from separate platforms.
We shouldn't compute `platforms` again in a different way. Instead
we pass the value we already computed, and then we perform a
consistency check whenever `len(lock_specs) > 0`.
@maresb
Copy link
Author

maresb commented Mar 5, 2023

See the commit annotations for explanations.

@maresb
Copy link
Author

maresb commented Mar 5, 2023

Tests: conda#384

@srilman srilman merged commit 3ce07dc into srilman:lockspec-platform-mapping Mar 5, 2023
@maresb maresb deleted the lockspec-platform-mapping-2 branch March 5, 2023 22:56
srilman pushed a commit that referenced this pull request Nov 24, 2023
…file

Add test for PipRepository aggregation
srilman pushed a commit that referenced this pull request Dec 21, 2023
* Refactor manylinux tag logic

* Run doctests

* Remove unused logger

* Clarify that legacy manylinux tags aren't glibc versions
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