-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
chore: add aarch64-apple-darwin builds to ci #21243
Conversation
.github/workflows/ci.generate.ts
Outdated
"(github.repository == 'denoland/deno' &&", | ||
"(github.ref == 'refs/heads/main' ||", | ||
"startsWith(github.ref, 'refs/tags/'))))", | ||
"(github.repository == 'denoland/deno')))", |
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.
The skip_pr
flag already controls this, so this change allows us to test release builds without pushing to main
@@ -718,7 +738,7 @@ const ci = { | |||
name: "Test debug (fast)", | |||
if: [ | |||
"matrix.job == 'test' && matrix.profile == 'debug' &&", | |||
"!startsWith(matrix.os, 'ubuntu')", | |||
"(startsWith(github.ref, 'refs/tags/') || !startsWith(matrix.os, 'ubuntu'))", |
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.
We were skipping test debug on ubuntu for tag builds
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 believe this was done on purpose to limit CI run time for tags.
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.
It's a bit odd because we run the fast tests on every other platform, but ubuntu (the fastest runner) doesn't. It kind of feels like an accidental bit of logic.
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.
Exciting, let's try this!
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.
We need to prevent this from running with macos-13-xlarge on PRs.
Oh wait, that seems to be the case in the workflow run. I forget how this code works. |
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.
LGTM! Thanks for doing this.
profile: "release", | ||
// TODO(mmastrac): We don't want to run this M1 runner on every main commit because of the expense. | ||
skip: | ||
"${{ github.event_name == 'pull_request' || github.ref == 'refs/heads/main' }}", |
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.
Nice! In the future, perhaps we could have this also run infrequently on main.
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.
We might be able to do something silly like "if commit ID ends in zero, run aarch64 build"
This is a prerequisite to automatic code signing.
This is a prerequisite to automatic code signing. (cherry picked from commit 93c4c1a)
This is a prerequisite to automatic code signing.
This is a prerequisite to automatic code signing.