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

Support v1 recipes in CrossRBaseMigrator #3836

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Mar 7, 2025

Description:

Add support for v1 recipes in CrossRBaseMigrator. Here the logic
made it easy to output a single combined condition:

    - if: build_platform != target_platform
      then:
        - cross-r-base ${{ r_base }}
        - r-rlang

Checklist:

  • Pydantic model updated or no update needed

Cross-refs, links to issues, etc:

Part of bug #3642

mgorny added 2 commits March 7, 2025 17:39
`CrossRBaseMigrator` is not actually using anything
from `CrossCompilationMigratorBase`, besides `post_migration` attribute,
and calling the superclass `filter()` method yields incorrect results.
Make the class inherit `MiniMigrator` instead to make it more consistent
with other minimigrators.
Add support for v1 recipes in `CrossRBaseMigrator`.  Here the logic
made it easy to output a single combined condition:

```yaml
    - if: build_platform != target_platform
      then:
        - cross-r-base ${{ r_base }}
        - r-rlang
```

Part of bug regro#3642
Comment on lines +356 to +361
[
" " * nspaces
+ "- if: build_platform != target_platform\n",
" " * nspaces + " then:\n",
" " * nspaces + " - cross-r-base ${{ r_base }}\n",
]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWICS we could just append all the lines as a single element here, but I've figured out that since it's a list of lines by design, it's cleaner to add them separately.

Copy link

codecov bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.98%. Comparing base (277fe85) to head (f2059b5).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
conda_forge_tick/migrators/cross_compile.py 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3836      +/-   ##
==========================================
- Coverage   78.38%   76.98%   -1.40%     
==========================================
  Files         138      138              
  Lines       15194    15204      +10     
==========================================
- Hits        11910    11705     -205     
- Misses       3284     3499     +215     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@beckermr beckermr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want the flaky decorator inside the pytest parameterize decorator again.

@mgorny
Copy link
Contributor Author

mgorny commented Mar 7, 2025

I think we want the flaky decorator inside the pytest parameterize decorator again.

Sorry about that. Remember about it for Python, failed to remember here.

@beckermr
Copy link
Contributor

beckermr commented Mar 7, 2025

We can merge this despite the target_platform versus host_platform issue since we'll need to change that everywhere including rattler later, and this won't happen for a while.

Is that OK @isuruf?

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