Skip to content

Conversation

@JonoPrest
Copy link
Collaborator

@JonoPrest JonoPrest commented Dec 2, 2025

Working workflow that uses our own docker base images (with node 22 and latest rust)

Summary by CodeRabbit

  • Chores
    • Optimized Docker build process with improved authentication handling for container images.
    • Streamlined Rust toolchain installation in Alpine Docker builds.
    • Updated GitHub Actions workflows to enhance build reliability and security permissions.

✏️ Tip: You can customize this high-level summary in your review settings.

@JonoPrest JonoPrest requested a review from JasoonS December 2, 2025 17:19
@coderabbitai
Copy link

coderabbitai bot commented Dec 2, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This PR updates GitHub Actions workflows and Docker build configuration to modernize the CI/CD pipeline. It switches the Alpine Dockerfile from apk-based Rust toolchain installation to rustup-init.sh, adds GitHub Container Registry authentication to the npm publish workflow, and applies minor formatting adjustments across workflow files.

Changes

Cohort / File(s) Summary
GitHub Actions Formatting
​.github/workflows/build-docker-images.yaml
Converted docker path patterns and options from single quotes to double quotes. No functional changes to build logic.
npm Publish Workflow Enhancement
​.github/workflows/publish_to_npm.yaml
Added GHCR authentication and image pull steps in docker build path (gated by matrix.settings.docker). Removed explicit rustup update stable commands from docker build steps. Added packages: read environment permission.
Alpine Dockerfile Toolchain Migration
docker/alpine.Dockerfile
Replaced multi-step apk-based Rust toolchain setup with rustup-init.sh download and execution. Removed edge/testing repo configuration. Retained build dependencies but reorganized installation sequence. Added cleanup steps for extracted archives and temporary scripts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Dockerfile toolchain migration: Verify rustup-init.sh approach maintains consistent Rust version pinning and target installations across Alpine builds
  • GHCR authentication flow: Confirm authentication gates are correctly applied and don't introduce race conditions or unnecessary image pulls
  • Workflow dependency changes: Validate that removing explicit rustup updates doesn't introduce toolchain inconsistencies between docker and non-docker build paths

Possibly related PRs

  • Add windows arm and bump version #49: Modifies the same .github/workflows/build-docker-images.yaml and docker/alpine.Dockerfile files, indicating overlapping build infrastructure updates
  • Use stable toolchain for job #42: Adds rustup update stable lines to the publish workflow while this PR removes them from docker build steps—directly related to toolchain management strategy

Suggested reviewers

  • JasoonS

Poem

🐰 With rustup-init we build so fast,
Authentication gates hold steadfast,
Alpine's toolchain now runs clean,
Workflows polished, quotes supreme,
Docker images dance with delight! 🎉

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jp/release-windows-arm2

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ece1b3 and e06d7d2.

📒 Files selected for processing (3)
  • .github/workflows/build-docker-images.yaml (4 hunks)
  • .github/workflows/publish_to_npm.yaml (2 hunks)
  • docker/alpine.Dockerfile (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

This reverts commit 2a263e6.
@JonoPrest JonoPrest merged commit 6652fc9 into main Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants