fix: use docker image instead#1932
Conversation
WalkthroughThe pull request updates the GitHub Actions workflow for building the NAPI project on the Changes
Sequence Diagram(s)sequenceDiagram
participant Actions as "GitHub Actions"
participant Docker as "Docker Image (napi-aarch64-linux-gnu)"
Actions->>Docker: Pull Docker Image
Docker-->>Actions: Provide dependencies
Actions->>Actions: Run yarn build in crates/napi
Actions->>Actions: Strip .node files
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1932 +/- ##
=======================================
Coverage 87.13% 87.13%
=======================================
Files 99 99
Lines 15877 15877
=======================================
Hits 13835 13835
Misses 2042 2042 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/napi.yml (1)
70-70: Docker Image Update for aarch64-unknown-linux-gnu TargetThe Docker image on line 70 now uses
ghcr.io/ast-grep/ast-grep/napi-aarch64-linux-gnu:latest. This change simplifies the build process by relying on a pre-configured image that should include all necessary dependencies (e.g., Node.js and Rust), thereby removing the need for manual installation commands. Please verify that:
- The new image indeed provides all required toolchain components (including the
aarch64-unknown-linux-gnu-striputility used in line 75).- Using the
latesttag meets your reproducibility requirements; consider pinning to a specific version if build consistency is critical.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/napi.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: stable - i686-pc-windows-msvc - node@18
- GitHub Check: stable - aarch64-pc-windows-msvc - node@18
- GitHub Check: stable - x86_64-pc-windows-msvc - node@18
940afb7 to
fc65c1b
Compare
fc65c1b to
1cca627
Compare
|
it compiles 
|
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
.github/workflows/napi.yml:70
- Verify that the new Docker image includes all the necessary dependencies (e.g., Node.js and the Rust toolchain) since the installation steps have been removed.
docker: ghcr.io/ast-grep/ast-grep/napi-aarch64-linux-gnu:latest
.github/workflows/napi.yml:75
- Confirm that sourcing the Cargo environment here is still required with the updated Docker image, ensuring that the Rust toolchain is properly configured.
. "$HOME/.cargo/env"
fix #1930
Summary by CodeRabbit
aarch64-unknown-linux-gnutarget and removed unnecessary installation commands.