Conversation
adespawn
force-pushed
the
arm-release
branch
2 times, most recently
from
ca9b1ac to
b281eea
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds ARM (aarch64-unknown-linux-gnu) architecture support to the release process. The changes enable building, testing, and publishing ARM64 binaries alongside the existing x86_64 binaries.
Changes:
- Added aarch64-unknown-linux-gnu as a build target in package.json
- Created a new build job for aarch64 architecture using ubuntu-24.04-arm runners
- Added an aarch64 test job that runs examples (integration tests are not run on ARM due to platform limitations)
- Updated the publish job to include ARM artifacts and wait for ARM tests to complete
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| package.json | Added aarch64-unknown-linux-gnu to the napi targets array for multi-architecture support |
| .github/workflows/release.yml | Added build and test jobs for ARM64 architecture, removed outdated comments, and updated publish job dependencies and artifact handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@wprzytula Some thought here: since we are releasing multiple packages (1 main + 1 / architecture) maybe we want to have a separate step that releases all the packages in the Doing dry run should be able to catch most of the problems from our end, so (in theory) the only chance the full run could fail later, would be infrastructure problems: ex. npm dying in the middle of release process |
| uses: actions/download-artifact@v6 | ||
| with: | ||
| path: artifacts | ||
| merge-multiple: true |
There was a problem hiding this comment.
Adding merge-multiple: true changes the download behavior: instead of creating subdirectories per artifact name, all files are merged flat into artifacts/. Does npm run artifacts expects/ready for this change?
There was a problem hiding this comment.
Before I was downloading a single artifact, which was not placed into the specific driectory. Now when I download multiple artifacts I need merge-multiple: true to keep the old behaviour
There was a problem hiding this comment.
I've updated the release to have the arm in the matrix
There was a problem hiding this comment.
@nikagra I just commented in the wrong place 🤦
| build-x86_64-unknown-linux-gnu: | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: |
There was a problem hiding this comment.
Any reason for having matrix here and not for build-aarch64-unknown-linux-gnu
There was a problem hiding this comment.
The upload step is slightly different: I upload 1 file in arm build, and 3 files in x64.
As a second thought, the syntax for github actions should allow to define what files to upload in the matrix, so maybe I can use it here.
There was a problem hiding this comment.
Should I wait for you updating this config?
There was a problem hiding this comment.
It's already done.
adespawn
force-pushed
the
arm-release
branch
4 times, most recently
from
cee36df to
a19b65e
Compare
|
I've tested all the changes in the dry mode: https://github.com/scylladb/nodejs-rs-driver/actions/runs/22901912473 |
| build-x86_64-unknown-linux-gnu: | ||
| build: | ||
| strategy: | ||
| fail-fast: false |
There was a problem hiding this comment.
I want to know all the steps that fail, so I can fix them in one go, rather than needing to re-run if two things fail
There was a problem hiding this comment.
Then you should leave this, not delete it...
Yes, sounds wise. |
Add arm to release process. This includes building, and then testing the built binary on examples (with a single node version).
I've testes this CI with
--dry-runoption, to ensure everything works correctly.