-
Notifications
You must be signed in to change notification settings - Fork 19
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
re-organize build matrices #177
Conversation
/merge |
haha thanks @msarahan . @ajschmidt8 could you merge this? |
let's see if I have any power here... /merge |
Thanks @bdice ! |
I don’t think this repo uses /merge commands? I’m not sure if that’s intentional or an oversight. Also FYI, the comment must be only /merge and not a message containing /merge. @msarahan’s attempt would not have worked because it had content before the /merge. |
Thanks for that! For the record, I didn't have the green |
Contributes to rapidsai/build-planning#3
Related to rapidsai/build-planning#5
Proposes re-organizing the build matrices.
Specifically:
amd64
andarm64
jobsBenefits of these changes
I believe the proposed changes here will make changes like "add a new CUDA version" or "drop support for a Python version" easier to author and review.
Inspired by 2 recent experiences I've had where I proposed changes to the build matrices and both I and reviewers found them difficult to assess:
I hope this re-organization will make it easier to determine at a glance whether matrices are achieving goals like:
{major}.{minor}
RAPIDS wants to support"Notes for Reviewers
To be clear, this is is only a moving-code-around PR.
It does not propose any changes to the build matrices for any workflows. If you see any places where this PR would lead to adding a new type of job or dropping a job that's currently being run, that's a mistake.
How I tested this
Opened rapidsai/cudf#15111 to test.
The matrix of jobs there (https://github.com/rapidsai/cudf/actions/runs/7994808681/job/21834408169?pr=15111) looks identical to the most recent successful run from some other PR targeting
branch-24.04
(https://github.com/rapidsai/cudf/actions/runs/7992742449).Also spot-checked the logs to confirm that we were getting the right runner architecture, Python version, CUDA version, and operating system.
Some of the job names did change... I think because we don't specify
name:
explicitly in the workflows here, so they're auto-generated based on the matrix entries.(left = before, right = as of this PR)
I don't think that should affect the correctness of branch protections, since we have this mechanism:
shared-workflows/.github/workflows/pr-builder.yaml
Lines 1 to 9 in da626b6
That's discussed over in #118.