-
Notifications
You must be signed in to change notification settings - Fork 84
feat: make create_migration_yaml_creator
v1 compatible
#3970
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.
Pull Request Overview
This PR updates the conda-forge tick migration logic to make create_migration_yaml_creator v1‐compatible by adopting a new Pydantic‐style schema extraction function.
- Replaces multiple instances of get_keys_default with get_recipe_schema_version in utility functions
- Adds tests for get_recipe_schema_version and adjusts migration creator logic to use a consistent feedstock naming approach
- Refactors the run_exports processing in migrators and updates related test fixtures
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/test_utils.py | Added tests for get_recipe_schema_version functionality |
tests/test_make_migrators.py | Added tests for migration yaml creator handling v0 and v1 recipes |
tests/test_files_make_migrators/test_conda_build_config.yaml | New sample config used for testing |
conda_forge_tick/utils.py | Introduces get_recipe_schema_version replacing get_keys_default |
conda_forge_tick/migrators/version.py | Refactored to use get_recipe_schema_version |
conda_forge_tick/migrators/staticlib.py | Replaced get_keys_default with get_recipe_schema_version |
conda_forge_tick/migrators/core.py | Updated schema version retrieval using get_recipe_schema_version |
conda_forge_tick/make_migrators.py | Updated logic to use feedstock_name consistently and refactored pinning tests |
Files not reviewed (2)
- tests/test_files_make_migrators/conda-forge-pinning_node_attrs.json: Language not supported
- tests/test_files_make_migrators/test_graph.json: Language not supported
Comments suppressed due to low confidence (1)
tests/test_utils.py:167
- [nitpick] Consider adding parentheses to clarify the conditional expression for better readability, e.g., assert get_recipe_schema_version(attrs) == (version if version is not None else 0).
assert get_recipe_schema_version(attrs) == version if version is not None else 0
Thanks a bunch for this PR and the tests! This one is going to conflict pretty badly with a big refactor I am working on in #3897. That refactor is nearly ready to merge, but not quite there yet. What do we think is the best way to proceed? |
Where exactly do you think the conflict happens? After having a brief look at your PR, it looks to me that you currently have only some minor changes to the debug output of |
The test file is the main spot I think? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3970 +/- ##
==========================================
+ Coverage 77.30% 78.16% +0.86%
==========================================
Files 140 141 +1
Lines 15619 15726 +107
==========================================
+ Hits 12075 12293 +218
+ Misses 3544 3433 -111 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I don't think this is too much of an issue. Either branch should be able to merge and resolve the problems if merged last. The test resources here are mostly just a copy of the current cf-graph files. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description:
Checklist:
Cross-refs, links to issues, etc:
Cc @pavelzw @xhochy