-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
switch to r-lib/actions/setup-r-dependencies@v2
#274
base: main
Are you sure you want to change the base?
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.
Looks good
Unit Tests Summary3 tests 3 ✅ 6s ⏱️ Results for commit b0b08e0. ♻️ This comment has been updated with latest results. |
These are good changes but not complete as of now. |
I'll be testing with this teal PR insightsengineering/teal#1466 |
by |
All where necessary. Obviously these live in our org and include also tern and family etc. |
…nor in r-hub/actions/setup-deps
@pawelru so far I adjusted possible inputs that are passed to |
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.
This is a good one and I'm ok with that but please note that it's a breaking change for all the packages that are still using lookup-refs
parameter! Hence not approving formally but just saying it's good but others are not ready.
Co-authored-by: Pawel Rucki <[email protected]> Signed-off-by: Marcin <[email protected]>
…tsengineering/r.pkg.template into rlib_setup_r_dependencies
Signed-off-by: Marcin <[email protected]>
A test PR in |
OK. This is good to go. Please make sure that all (!) the upstream packages follows - in particular: they might be passing parameters that you are about to remove! This most likely would fail the jobs. |
For now it runs smoothly in the PR created in I also created a card to track down all other packages insightsengineering/nestdevs-tasks#103 |
@@ -158,12 +152,11 @@ jobs: | |||
- name: Setup R dependencies 🎦 |
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.
Adding a comment, so it can not be merged.
It should not be merged before other packages are not suited for changes introduced in this PR.
Other packages are tracked in this card insightsengineering/nestdevs-tasks#103
Part of https://github.com/insightsengineering/coredev-tasks/issues/609