-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
Enable native AArch64 Ubuntu CI jobs #127584
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
Conversation
The Arm runners are now enabled for the JIT builds. We exclude the Arm runners for forks fro both Tests and JIT builds.
This is the continuation of #125786 |
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.
JIT changes look good, thanks for tackling this! CI-driven development is no fun, I know.
Co-authored-by: Brandt Bucher <[email protected]>
Hmm, I do find it somewhat misleading that the jobs just "passes" immediately (well, not quite immediately, it takes about 11s to spin up and checkout), and would prefer to skip it entirely. And when we do run, we're skipping three irrelevant jobs, and there's nothing much special we're sharing in the setup config to benefit from it (just checkout and setup-python): And I did like the refactored one had more readable job names and grouped the three operating systems in the summary: Compared to this: But from a practical point of view it's not such a big deal, and the important thing is to start testing the new runners, so I've approved! Thanks all for your work! |
But the alternative is just not running the job at all, right? Maybe when this happens we can just echo one of GitHub's magic warning messages to stdout so at least a yellow box appears? I'm not strongly opposed to refactoring if many others feel that way, I just didn't think that this particular change warranted rewriting the whole file and adding a bunch of new logic. I personally find the current structure a easier to follow:
If rewriting it so every OS is its own separate set of jobs would help in some real way, I'm probably okay with it in its own issue/PR. But simpler things like changing the names are easy enough to do in the current file. And it seems like we'll need "clever" tricks for handling the paid runners anyways. |
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.
Much simpler! Thanks Diego!
PR #125786 was back ported to 3.13. I don't think we need this for 3.13 as we don't build the JIT in this version. Thoughts? |
Co-authored-by: Brandt Bucher <[email protected]>
The Arm runners are now enabled for the JIT builds.
We exclude the Arm runners on forks for both Tests and JIT jobs.