feat: make create_migration_yaml_creator v1 compatible#3970
feat: make create_migration_yaml_creator v1 compatible#3970beckermr merged 6 commits intoregro:mainfrom
create_migration_yaml_creator v1 compatible#3970Conversation
There was a problem hiding this comment.
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