-
Notifications
You must be signed in to change notification settings - Fork 79
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
base: main
Are you sure you want to change the base?
Conversation
`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
[ | ||
" " * nspaces | ||
+ "- if: build_platform != target_platform\n", | ||
" " * nspaces + " then:\n", | ||
" " * nspaces + " - cross-r-base ${{ r_base }}\n", | ||
] |
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.
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.
Codecov ReportAttention: Patch coverage is
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. 🚀 New features to boost your workflow:
|
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.
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. |
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? |
Description:
Add support for v1 recipes in
CrossRBaseMigrator
. Here the logicmade it easy to output a single combined condition:
Checklist:
Cross-refs, links to issues, etc:
Part of bug #3642