ci: upload WASM and download in other workflows #147
Conversation
WalkthroughThe pull request modifies several GitHub workflow configurations to change their triggering mechanisms to Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
.github/workflows/wasmstan.yml (1)
32-35: Consider enhancing artifact upload configuration.To improve reliability for downstream workflows, consider adding:
- Artifact retention period
- Compression settings for the WASM file
- uses: actions/upload-artifact@v4 with: name: prophet-wasmstan.wasm path: components/cpp/prophet-wasmstan.wasm + retention-days: 1 + compression-level: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/release-plz.yml(1 hunks).github/workflows/rust.yml(2 hunks).github/workflows/wasmstan.yml(1 hunks)
| workflow_run: | ||
| workflows: | ||
| - prophet-wasmstan | ||
| types: | ||
| - "completed" |
There was a problem hiding this comment.
Add conclusion: success to the workflow trigger.
The workflow should only run when the prophet-wasmstan workflow succeeds, not when it fails. This is especially important for a release workflow.
Apply this diff to ensure releases only happen after successful WASM builds:
workflow_run:
workflows:
- prophet-wasmstan
types:
- "completed"
+ conclusion: success
branches:
- main📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| workflow_run: | |
| workflows: | |
| - prophet-wasmstan | |
| types: | |
| - "completed" | |
| workflow_run: | |
| workflows: | |
| - prophet-wasmstan | |
| types: | |
| - "completed" | |
| conclusion: success |
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: prophet-wasmstan.wasm | ||
| path: components/cpp/prophet-wasmstan.wasm |
There was a problem hiding this comment.
💡 Codebase verification
The artifact path in the workflow needs to be updated
The current path components/cpp/prophet-wasmstan.wasm in the workflow is incorrect. According to the build configuration in components/justfile, the WASM artifact is generated as prophet-wasmstan-component.wasm in the cpp/prophet-wasmstan directory. The correct path should be:
path: components/cpp/prophet-wasmstan/prophet-wasmstan-component.wasm- The build process generates
prophet-wasmstan-component.wasmas shown in thebuildtarget of the justfile - The README also confirms this is the expected output filename
- The rust.yml workflow attempts to download this artifact with a different name, indicating a mismatch in the artifact configuration
🔗 Analysis chain
LGTM! Verify the artifact path exists.
The artifact upload step is correctly configured using the latest version of the upload-artifact action.
Let's verify the artifact path exists in the repository:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the WASM artifact path exists and is generated during the build
# Expected: Find references to the WASM file in build scripts or configuration
# Check for references to the WASM file path in build scripts
rg -l "prophet-wasmstan\.wasm" components/
# Check if there are any build/compilation steps that generate this file
fd -t f "just.*" components/ -x grep -l "prophet-wasmstan\.wasm" {} \;
Length of output: 120
Script:
#!/bin/bash
# Let's try a broader search to understand the WASM build setup
# Check for any WASM-related build configurations
rg -l "\.wasm" components/cpp/
# Look for build scripts or configuration files in cpp component
fd -t f "(CMakeLists\.txt|Makefile|\.cmake|build\.rs|\.toml)" components/cpp/
# Check for any justfile in the repository
fd justfile
# Look for workflow files that might reference this artifact
rg -l "prophet-wasmstan" .github/workflows/
Length of output: 333
Script:
#!/bin/bash
# Let's check the content of relevant files to understand the build process
# Check the README for WASM build instructions
cat components/cpp/prophet-wasmstan/README.md
# Check the justfiles for WASM build targets
cat components/justfile
cat justfile
# Check other workflows that reference this component
cat .github/workflows/rust.yml
Length of output: 8743
| - uses: actions/download-artifact@v4 | ||
| with: | ||
| name: prophet-wasmstan.wasm | ||
| path: crates/augurs-prophet/prophet-wasmstan.wasm |
There was a problem hiding this comment.
Move artifact download after checkout.
The artifact download step is positioned before the repository checkout, which could cause issues:
- The target directory structure doesn't exist yet
- The downloaded artifact might be affected by the subsequent checkout
Apply this change to fix the ordering:
- - uses: actions/download-artifact@v4
- with:
- name: prophet-wasmstan.wasm
- path: crates/augurs-prophet/prophet-wasmstan.wasm
- name: Checkout sources
uses: actions/checkout@v4
- name: Install Rust toolchain
uses: moonrepo/setup-rust@v1
+ - name: Download WASM artifact
+ uses: actions/download-artifact@v4
+ with:
+ name: prophet-wasmstan.wasm
+ path: crates/augurs-prophet/prophet-wasmstan.wasm📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/download-artifact@v4 | |
| with: | |
| name: prophet-wasmstan.wasm | |
| path: crates/augurs-prophet/prophet-wasmstan.wasm | |
| - name: Checkout sources | |
| uses: actions/checkout@v4 | |
| - name: Install Rust toolchain | |
| uses: moonrepo/setup-rust@v1 | |
| - name: Download WASM artifact | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: prophet-wasmstan.wasm | |
| path: crates/augurs-prophet/prophet-wasmstan.wasm |
| workflow_run: | ||
| workflows: | ||
| - prophet-wasmstan | ||
| types: | ||
| - "completed" |
There was a problem hiding this comment.
💡 Codebase verification
Sequential workflow dependency is justified but can be optimized
The sequential dependency on prophet-wasmstan workflow is necessary but only for the test job. The current setup has these inefficiencies:
- Only the
testjob requires the WASM artifact (prophet-wasmstan.wasm) check,fmt, andclippyjobs are blocked unnecessarily
Recommend splitting this into two workflows:
- A parallel workflow for
check,fmt, andclippythat runs on push/PR - Keep the current sequential workflow only for
testjob that depends onprophet-wasmstan
This will optimize CI time while maintaining the required dependencies.
🔗 Analysis chain
Verify the sequential workflow dependency requirement.
The workflow now runs sequentially after prophet-wasmstan completes, which could increase overall CI time. Consider:
- Whether all jobs need the WASM artifact (currently only
testjob uses it) - If jobs like
check,fmt, andclippycould run in parallel withprophet-wasmstan
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the prophet-wasmstan workflow exists and its artifact usage
# Test 1: Check if the prophet-wasmstan workflow exists
echo "Verifying prophet-wasmstan workflow existence:"
fd -p ".github/workflows/*" -x grep -l "name:.*prophet-wasmstan"
# Test 2: Check which jobs/steps use the WASM artifact
echo -e "\nChecking WASM artifact usage in test commands:"
rg "prophet-wasmstan\.wasm" -A 3
Length of output: 313
Script:
#!/bin/bash
# Let's try a broader search to understand the workflow dependencies and WASM usage
# Check all workflow files
echo "=== All GitHub Workflow Files ==="
fd -p ".github/workflows/"
# Look for any WASM-related content in workflows
echo -e "\n=== WASM-related content in workflows ==="
rg -i "wasm" .github/workflows/
# Check for artifact download/usage in the rust.yml
echo -e "\n=== Artifact usage in rust.yml ==="
cat .github/workflows/rust.yml
# Look for any prophet-wasmstan references
echo -e "\n=== prophet-wasmstan references ==="
rg -i "prophet-wasmstan" .github/workflows/
Length of output: 3506
Summary by CodeRabbit
New Features
prophet-wasmstanworkflow.Bug Fixes
pushandpull_requestevents in Rust workflow.