Upload flake-iter to S3 #39
Conversation
|
Warning Rate limit exceeded@lucperkins has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughRemoves an old CI workflow, adds a new multi-target build-and-release workflow with artifact upload to GitHub and S3, updates Nix formatting and flake outputs/refactoring, and exposes CLI version metadata via a Clap attribute. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions (workflow)
participant Runner as Runner (host)
participant Determ as Determinate Nix / Build cache
participant Artif as Artifact storage (GitHub)
participant S3 as S3 bucket
GH->>Runner: checkout code\nsetup Determinate Nix\nset FlakeHub cache
Runner->>Determ: nix build -L (matrix target)
Determ-->>Runner: build result (./result)
Runner->>Artif: prepare artifact (copy result/bin -> artifacts/)
Runner->>Artif: upload artifact (GitHub Actions upload)
GH->>S3: trigger upload job (downloads artifacts)
GH->>S3: push artifacts using action with AWS secrets
S3-->>GH: confirm upload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
.github/workflows/build-and-upload.yml (1)
1-6: Missing workflownameattribute.Adding a
nameattribute improves visibility in the GitHub Actions UI, especially since this workflow is called by multiple release workflows.+name: Build and upload + on: workflow_call:.github/workflows/release-tag.yml (1)
5-6: Tag pattern may match unintended versions.The pattern
v*.*.*will also match tags likev1.2.3-beta,v1.2.3.4, orv1.2.3-rc1. If you want to match only strict semver tags, consider a more specific pattern.For stricter semver matching:
tags: - - v*.*.* + - 'v[0-9]+.[0-9]+.[0-9]+'Alternatively, if you need to support pre-release tags separately, keep the current pattern.
.github/workflows/release-pr.yml (2)
24-31: Consider scoping concurrency control to individual PRs.The current
concurrency: releasesetting means only one PR release can run at a time across all PRs. This could block concurrent PR releases unnecessarily.Apply this diff to scope concurrency to the specific PR:
- concurrency: release + concurrency: release-pr-${{ github.event.pull_request.number }}
36-37: Consider usingmkdir -pfor more robust directory creation.The current command chains
rm -rfandmkdir, which will fail if the removal fails for any reason.Apply this diff for a more robust approach:
- run: rm -rf ./artifacts && mkdir ./artifacts + run: mkdir -p ./artifactsNote: If you specifically need to ensure a clean directory, the current approach is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/actions/download-persist.yaml(1 hunks).github/workflows/build-and-upload.yml(1 hunks).github/workflows/determinate-ci.yml(0 hunks).github/workflows/release-branch.yml(1 hunks).github/workflows/release-pr.yml(1 hunks).github/workflows/release-tag.yml(1 hunks)flake.nix(3 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/determinate-ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-artifacts (aarch64-darwin, macos-latest-xlarge, flake-iter-ARM64-macOS)
- GitHub Check: build-artifacts (x86_64-linux, ubuntu-24.04, flake-iter-ARM64-Linux)
- GitHub Check: flake-check
- GitHub Check: rust-fmt-and-clippy
🔇 Additional comments (8)
flake.nix (1)
17-18: LGTM!The updated outputs signature using
{ self, ... }@inputs:is the idiomatic pattern for accessingselfin Nix flakes. The subsequent changes to useself.overlays.defaultandsrc = selfare cleaner than the previousinputs.selfapproach..github/workflows/release-branch.yml (1)
8-54: LGTM!The workflow structure is well-designed with proper job dependencies, concurrency control, and environment isolation. The artifact download steps correctly reference the expected platform combinations.
.github/workflows/release-tag.yml (1)
17-19: Consider adding a GitHub release creation step.The workflow has
contents: writepermission (for uploading to GitHub releases as noted), but there's no step that actually creates a GitHub release. If you intend to create releases, consider adding a step usingsoftprops/action-gh-releaseorgh release create.Is the GitHub release creation handled elsewhere, or should it be added to this workflow?
.github/workflows/build-and-upload.yml (1)
27-28: actions/checkout@v6 is correct and current.Version v6.0.1 is the latest release of actions/checkout, not v4. The code is already using the correct, up-to-date action.
.github/actions/download-persist.yaml (1)
23-23: Verifyactions/download-artifact@v6exists and fix composite action input syntax.The file uses
type: choicefor inputs (lines 8, 14), which is invalid for composite actions—usetype: stringinstead. Additionally, verify thatactions/download-artifact@v6is available; recent stable versions are v4 or earlier..github/workflows/release-pr.yml (3)
3-10: LGTM!The PR trigger configuration appropriately covers the necessary event types for a build-and-release workflow.
12-22: Conditional logic correctly restricts artifact uploads to intra-repo PRs with the 'upload to s3' label.The build job properly gates artifact uploads by checking that the PR originates from the main repository and that the 'upload to s3' label is present. The reusable workflow reference to
.github/workflows/build-and-upload.ymlis valid, and the approach promotes DRY principles.
39-58: Local action is properly implemented and supports all specified platforms.The
./.github/actions/download-persistaction is a well-structured composite action that supports the three platforms in this workflow: macOS ARM64, Linux x64, and Linux ARM64. The action correctly constructs artifact names using the pattern{name}-{arch}-{os}and persists downloads to platform-specific directories. No issues found.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/build-and-release.yml (4)
36-38: Consider making the build system explicit.The build command relies on native builds without explicitly specifying the target system. While this should work since runners match target architectures, consider either:
- Adding a comment explaining the native build approach
- Or explicitly passing the system:
nix build -L .#packages.${{ matrix.systems.nix-system }}.default
31-34: Pin action versions instead of using @main.Using
@mainfor actions can lead to unexpected breakage and security concerns. Consider pinning to specific versions or release tags.Apply this approach to pin versions:
- - name: Install Determinate Nix - uses: DeterminateSystems/determinate-nix-action@main - - - name: Set up FlakeHub Cache - uses: DeterminateSystems/flakehub-cache-action@main + - name: Install Determinate Nix + uses: DeterminateSystems/determinate-nix-action@v1.2.3 # Use actual latest stable version + + - name: Set up FlakeHub Cache + uses: DeterminateSystems/flakehub-cache-action@v1.0.0 # Use actual latest stable version
65-72: Pin action version instead of using @main.Similar to the earlier actions,
DeterminateSystems/push-artifact-ids@mainshould be pinned to a specific version for reproducibility and security.- name: Upload to S3 - uses: DeterminateSystems/push-artifact-ids@main + uses: DeterminateSystems/push-artifact-ids@v1.0.0 # Use actual latest stable version with:
59-63: Updateactions/download-artifactto the latest version.actions/download-artifact@v7 is the current version, which runs on Node.js 24 and requires a minimum Actions Runner version of 2.327.1. Update to v7 to align with the latest release:
- name: Download artifacts directory - uses: actions/download-artifact@v7 + uses: actions/download-artifact@v7 with: name: artifacts path: ./artifacts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-and-release.yml(1 hunks).github/workflows/nix.yml(1 hunks)flake.nix(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: flake-check
- GitHub Check: rust-fmt-and-clippy
🔇 Additional comments (7)
flake.nix (4)
18-21: LGTM: Clean refactor for clarity.The explicit
selfbinding andlibextraction reduce verbosity and improve readability. The changes are applied consistently throughout the file (lines 30, 36, 57, 60, 103).
33-33: LGTM: System parameter enables formatter reference.Adding
systemtoforEachSupportedSystemoutputs is necessary for theself.formatter.${system}reference on line 79.Also applies to: 68-68
98-98: LGTM: Future-proofs the function signature.Adding
...to the packages function signature allows for future extensibility, though currently onlypkgsis used. This maintains consistency with the devShells pattern.
79-79: The switch to nixfmt as the formatter is well-supported. The official Nix formatter nixfmt is now stable and available as pkgs.nixfmt, and nixfmt includes a check flag for use in CI. No additional verification needed..github/workflows/nix.yml (1)
21-21: LGTM: Consistent formatter switch.The workflow correctly switches to
nixfmtto align with theflake.nixformatter attribute change. This maintains consistency across the repository..github/workflows/build-and-release.yml (2)
1-6: LGTM!The workflow triggers are appropriate for a build and release pipeline, covering manual dispatch, PR validation, and main branch builds.
8-25: LGTM!The matrix configuration correctly maps target architectures to appropriate runners for native builds. The permissions are properly configured for OIDC authentication.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.