Changed printing of simulation results #397
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe changes in this pull request involve modifications to two workflow files, Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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: 2
🧹 Outside diff range and nitpick comments (3)
.github/workflows/build.yml (2)
86-94: LGTM! Consider using heredoc for better readability.The logic for creating the merge artifact is correct. The condition and JSON structure are well-defined.
Consider using heredoc for better readability:
- - name: Save merge artifact - if: github.event_name != 'pull_request' - run: | - mkdir -p ./artifact - printf '{ - "is_pr": false, - "pr_id": -1 - }' >> ./artifact/artifact.json + - name: Save merge artifact + if: github.event_name != 'pull_request' + run: | + mkdir -p ./artifact + cat << EOF > ./artifact/artifact.json + { + "is_pr": false, + "pr_id": -1 + } + EOF
86-110: Well-structured approach to artifact handling.The changes introduce a more robust artifact handling system:
- Clear separation between PR and merge artifacts through structured JSON
- Consistent artifact naming across workflows
- Type-safe approach with explicit boolean flags and IDs
This implementation provides better integration with the drive workflow and resolves the issue of identifying PR-triggered builds.
Consider documenting the artifact schema in the repository's documentation to help maintain consistency as the workflow evolves.
🧰 Tools
🪛 actionlint
99-99: shellcheck reported issue in this script: SC2016:info:2:8: Expressions don't expand in single quotes, use double quotes for that
(shellcheck)
.github/workflows/drive.yml (1)
48-59: Add error handling for unzip and JSON parsing operations.The current implementation could be more robust with proper error handling:
- Unzip operation should verify successful extraction
- JSON parsing should handle potential errors
Consider updating the script like this:
- name: Unzip artifact run: | - unzip artifact.zip + unzip artifact.zip || exit 1 + [ -f "./artifacts/artifacts.json" ] || exit 1 - name: Return artifact JSON id: return-artifact-json uses: actions/github-script@v6 with: script: | let fs = require('fs'); - let data = JSON.parse(fs.readFileSync('./artifacts/artifacts.json')); - return data; + try { + let data = JSON.parse(fs.readFileSync('./artifacts/artifacts.json')); + return data; + } catch (error) { + core.setFailed(`Failed to parse artifacts.json: ${error.message}`); + return null; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/build.yml(1 hunks).github/workflows/drive.yml(5 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build.yml
99-99: shellcheck reported issue in this script: SC2016:info:2:8: Expressions don't expand in single quotes, use double quotes for that
(shellcheck)
.github/workflows/drive.yml
94-94: shellcheck reported issue in this script: SC2028:info:1:6: echo may not expand escape sequences. Use printf
(shellcheck)
🔇 Additional comments (3)
.github/workflows/build.yml (1)
106-110: LGTM! Verify artifact path pattern.
The upload configuration is correct, using the latest stable version of the action with appropriate retention settings.
Let's verify the artifact path pattern:
✅ Verification successful
Artifact path pattern is correctly configured
The artifact path configuration is consistent with the workflow's logic:
- The
./artifact/directory is properly created usingmkdir -p ./artifact - The JSON file is correctly written to
./artifact/artifact.json - The upload step correctly references this path with
path: ./artifact/ - The artifact contains workflow-specific metadata that's properly generated based on the event type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the artifact path pattern is used consistently across workflows
# Expected: All references to this artifact should use the same path pattern
# Test: Search for artifact path patterns in workflow files
rg -g '*.yml' -g '*.yaml' --type yaml 'artifact.json|name: artifact'
Length of output: 70
Script:
#!/bin/bash
# Let's try a broader search to find any artifact-related configurations in workflows
rg -g '*.yml' -g '*.yaml' --type yaml 'artifact' -A 3 -B 3
Length of output: 58
Script:
#!/bin/bash
# Also check if the ./artifact/ directory is referenced elsewhere
rg -g '*.yml' -g '*.yaml' --type yaml './artifact/' -A 2 -B 2
Length of output: 61
Script:
#!/bin/bash
# Check for any upload-artifact action usage
rg -g '*.yml' -g '*.yaml' --type yaml 'actions/upload-artifact' -A 3 -B 1
Length of output: 73
Script:
#!/bin/bash
# Let's check the workflow file directly to understand the context
cat .github/workflows/build.yml
Length of output: 3392
Script:
#!/bin/bash
# Also check for any download-artifact actions that might be consuming this artifact
rg -g '*.yml' -g '*.yaml' --type yaml 'actions/download-artifact'
Length of output: 65
Script:
#!/bin/bash
# Check if the artifact directory is created or referenced in the workflow
rg -g '*.yml' -g '*.yaml' --type yaml 'mkdir.*artifact|cp.*artifact'
Length of output: 70
.github/workflows/drive.yml (2)
Line range hint 23-46: LGTM! Artifact download implementation is robust.
The artifact download logic is well-implemented with proper error handling and clear messaging.
Line range hint 72-110: LGTM! Well-structured implementation of simulation results handling.
The changes effectively address the original issue by:
- Always printing results to action logs
- Creating a well-formatted markdown table
- Conditionally adding results as PR comments when appropriate
This implementation aligns perfectly with the PR objectives from issue #390.
🧰 Tools
🪛 actionlint
94-94: shellcheck reported issue in this script: SC2028:info:1:6: echo may not expand escape sequences. Use printf
(shellcheck)
🪛 yamllint
[error] 113-113: no new line character at the end of file
(new-line-at-end-of-file)
| - name: Save pull request artifact | ||
| if: github.event_name == 'pull_request' | ||
| env: | ||
| PR_ID: ${{ github.event.number }} | ||
| run: | | ||
| mkdir -p ./pr | ||
| echo $PR_ID > ./pr/pr_id | ||
| mkdir -p ./artifact | ||
| printf '{ | ||
| "is_pr": true, | ||
| "pr_id": $PR_ID | ||
| }' >> ./artifact/artifact.json |
There was a problem hiding this comment.
Fix shell script to properly expand environment variable.
The logic is correct, but there's a potential issue with variable expansion due to single quotes.
Apply this fix to ensure proper variable expansion:
- name: Save pull request artifact
if: github.event_name == 'pull_request'
env:
PR_ID: ${{ github.event.number }}
run: |
mkdir -p ./artifact
- printf '{
- "is_pr": true,
- "pr_id": $PR_ID
- }' >> ./artifact/artifact.json
+ cat << EOF > ./artifact/artifact.json
+ {
+ "is_pr": true,
+ "pr_id": $PR_ID
+ }
+ EOF📝 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.
| - name: Save pull request artifact | |
| if: github.event_name == 'pull_request' | |
| env: | |
| PR_ID: ${{ github.event.number }} | |
| run: | | |
| mkdir -p ./pr | |
| echo $PR_ID > ./pr/pr_id | |
| mkdir -p ./artifact | |
| printf '{ | |
| "is_pr": true, | |
| "pr_id": $PR_ID | |
| }' >> ./artifact/artifact.json | |
| - name: Save pull request artifact | |
| if: github.event_name == 'pull_request' | |
| env: | |
| PR_ID: ${{ github.event.number }} | |
| run: | | |
| mkdir -p ./artifact | |
| cat << EOF > ./artifact/artifact.json | |
| { | |
| "is_pr": true, | |
| "pr_id": $PR_ID | |
| } | |
| EOF |
🧰 Tools
🪛 actionlint
99-99: shellcheck reported issue in this script: SC2016:info:2:8: Expressions don't expand in single quotes, use double quotes for that
(shellcheck)
| run: | | ||
| echo "Simulation results\n" | ||
| echo "${{ steps.simulation-results.outputs.result }}" |
There was a problem hiding this comment.
Fix echo command to properly handle escape sequences.
The current echo command may not properly expand the newline character.
Update the command to use printf:
- echo "Simulation results\n"
- echo "${{ steps.simulation-results.outputs.result }}"
+ printf "Simulation results\n\n%s\n" "${{ steps.simulation-results.outputs.result }}"📝 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.
| run: | | |
| echo "Simulation results\n" | |
| echo "${{ steps.simulation-results.outputs.result }}" | |
| run: | | |
| printf "Simulation results\n\n%s\n" "${{ steps.simulation-results.outputs.result }}" |
🧰 Tools
🪛 actionlint
94-94: shellcheck reported issue in this script: SC2028:info:1:6: echo may not expand escape sequences. Use printf
(shellcheck)
https://github.com/una-auxme/paf into 390-bug-drive-action-does-not-comment-driving-results
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
.github/workflows/build.yml (2)
116-126: Clean up trailing spaces in the workflow file.The cache cleanup logic is correct, but there are some minor formatting issues to address.
Remove trailing spaces from lines 121 and 127:
- name: Clean up PR cache if: github.event_name == 'pull_request' run: | rm -rf /tmp/.buildx-cache/cache/test - mv /tmp/.buildx-cache/cache-new/test /tmp/.buildx-cache/cache/test - + mv /tmp/.buildx-cache/cache-new/test /tmp/.buildx-cache/cache/test + - name: Clean up merge cache if: github.event_name != 'pull_request' run: | rm -rf /tmp/.buildx-cache/cache/latest - mv /tmp/.buildx-cache/cache-new/latest /tmp/.buildx-cache/cache/latest - + mv /tmp/.buildx-cache/cache-new/latest /tmp/.buildx-cache/cache/latest +🧰 Tools
🪛 yamllint
[error] 121-121: trailing spaces
(trailing-spaces)
128-129: Add newline at end of file.The Docker image pruning logic is good for maintaining disk space on the self-hosted runner.
Add a newline at the end of the file:
- name: Prune all images older than 1 days from self-hosted runner - run: docker image prune -a --filter "until=24h" \ No newline at end of file + run: docker image prune -a --filter "until=24h" +🧰 Tools
🪛 yamllint
[error] 129-129: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/drive.yml (1)
Line range hint
72-91: Add error handling for simulation results parsing.While the table formatting logic is well-implemented, consider adding error handling for cases where the JSON structure might be unexpected.
script: | const fs = require('fs'); // read the simulation results const results = fs.readFileSync('./simulation_results.json', 'utf8'); - let resultsJson = JSON.parse(results); + let resultsJson; + try { + resultsJson = JSON.parse(results); + if (!resultsJson.values || !resultsJson.labels || + !Array.isArray(resultsJson.values) || + !Array.isArray(resultsJson.labels)) { + throw new Error('Invalid simulation results format'); + } + } catch (error) { + core.setFailed(`Failed to parse simulation results: ${error.message}`); + return ''; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/build.yml(3 hunks).github/workflows/drive.yml(5 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build.yml
93-93: shellcheck reported issue in this script: SC2016:info:2:8: Expressions don't expand in single quotes, use double quotes for that
(shellcheck)
.github/workflows/drive.yml
94-94: shellcheck reported issue in this script: SC2028:info:1:6: echo may not expand escape sequences. Use printf
(shellcheck)
🪛 yamllint
.github/workflows/build.yml
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 127-127: trailing spaces
(trailing-spaces)
[error] 129-129: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/drive.yml
[error] 113-113: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (8)
.github/workflows/build.yml (3)
38-49: LGTM! Well-structured caching configuration.
The Docker layer caching is properly configured with:
- Latest actions/cache@v4
- Separate paths for test and latest builds
- Effective cache key strategy with fallback
65-66: LGTM! Consistent cache configuration across builds.
The cache paths are properly configured for both test and production builds, using temporary locations for new cache data.
Also applies to: 82-83
89-113:
Fix shell script variable expansion in artifact creation.
The artifact handling logic is correct, but there's still a critical issue with variable expansion in the PR artifact creation due to single quotes.
Apply this fix to ensure proper variable expansion:
- name: Save pull request artifact
if: github.event_name == 'pull_request'
env:
PR_ID: ${{ github.event.number }}
run: |
mkdir -p ./artifact
- printf '{
- "is_pr": true,
- "pr_id": $PR_ID
- }' >> ./artifact/artifact.json
+ cat << EOF > ./artifact/artifact.json
+ {
+ "is_pr": true,
+ "pr_id": $PR_ID
+ }
+ EOFAlso apply similar changes to the merge artifact step for consistency:
- name: Save merge artifact
if: github.event_name != 'pull_request'
run: |
mkdir -p ./artifact
- printf '{
- "is_pr": false,
- "pr_id": -1
- }' >> ./artifact/artifact.json
+ cat << EOF > ./artifact/artifact.json
+ {
+ "is_pr": false,
+ "pr_id": -1
+ }
+ EOF🧰 Tools
🪛 actionlint
93-93: shellcheck reported issue in this script: SC2016:info:2:8: Expressions don't expand in single quotes, use double quotes for that
(shellcheck)
.github/workflows/drive.yml (5)
52-59: LGTM! Clean JSON parsing implementation.
The JSON parsing step is well-implemented using native Node.js functionality.
94-96: Fix echo command to properly handle escape sequences.
🧰 Tools
🪛 actionlint
94-94: shellcheck reported issue in this script: SC2028:info:1:6: echo may not expand escape sequences. Use printf
(shellcheck)
98-110: LGTM! Proper implementation of conditional PR commenting.
The implementation correctly addresses issue #390 by checking the is_pr flag before attempting to comment on the PR.
Line range hint 23-50: LGTM! Improved artifact handling implementation.
The changes to artifact handling are well-structured and more robust. The switch to JSON format provides better type safety and extensibility.
Let's verify the artifact structure in the build workflow:
✅ Verification successful
Artifact handling changes are correctly implemented
The verification confirms that:
- The build workflow correctly creates
artifact.jsonwith the expected structure - The artifact is properly uploaded with the name
artifact.json - The drive workflow's download and handling logic matches the build workflow's artifact format
- Both PR and non-PR scenarios are handled appropriately with correct JSON structures
The artifact handling changes are well-coordinated between the build and drive workflows, ensuring reliable data transfer between them.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the build workflow creates the expected artifact.json
# Test: Search for artifact.json creation in build workflow
rg -A 5 'artifact\.json' .github/workflows/build.yml
Length of output: 520
Script:
#!/bin/bash
# Let's verify the complete artifact creation logic in build.yml
rg -B 5 -A 2 'artifact\.json' .github/workflows/build.yml
Length of output: 588
112-113: Verify impact of aggressive Docker image pruning.
The change from 30 days to 1 day for Docker image pruning is significant. Please ensure this doesn't impact development workflows that might need older images.
✅ Verification successful
Let me check the Docker image usage patterns in these workflows to better understand the potential impact.
Let me check for any Docker build or pull operations to understand the image lifecycle better.
Let me check the docker-compose configuration to understand the complete Docker setup.
Docker image pruning change is safe to proceed
Based on the analysis of workflows:
- The repository uses Docker primarily for CI/CD purposes with docker-compose
- Images are built from Dockerfiles in the
build/docker/agent/directory - Workflows rebuild images during execution rather than relying on cached images
- The 24-hour pruning only affects the self-hosted runner's local storage
The aggressive pruning won't impact development since:
- CI/CD builds fresh images for each run
- Local development would use their own Docker environment, not the runner's
- No workflows depend on pre-existing or cached images
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other workflows that might reference Docker images
# Test: Search for Docker image references in workflows
rg -l 'docker.*image' .github/workflows/
Length of output: 98
Script:
#!/bin/bash
# Check Docker image usage patterns in both workflows
echo "=== drive.yml ==="
rg -A 5 'docker.*image' .github/workflows/drive.yml
echo -e "\n=== build.yml ==="
rg -A 5 'docker.*image' .github/workflows/build.yml
# Also check if there are any image caching or reuse patterns
echo -e "\n=== Checking for image caching patterns ==="
rg 'cache.*docker|docker.*cache' .github/workflows/
Length of output: 472
Script:
#!/bin/bash
# Check for Docker build and pull operations
echo "=== Checking Docker operations in drive.yml ==="
rg -B 5 -A 5 'docker (build|pull)' .github/workflows/drive.yml
echo -e "\n=== Checking Docker operations in build.yml ==="
rg -B 5 -A 5 'docker (build|pull)' .github/workflows/build.yml
# Also check for any docker-compose usage which might involve images
echo -e "\n=== Checking docker-compose usage ==="
rg -l 'docker-compose' .github/workflows/
Length of output: 506
Script:
#!/bin/bash
# Check docker-compose configuration and context
echo "=== Checking docker-compose context in drive.yml ==="
rg -B 5 -A 5 'docker-compose' .github/workflows/drive.yml
# Look for any docker-compose files in the repository
echo -e "\n=== Looking for docker-compose files ==="
fd 'docker-compose.ya?ml'
# Check for any Docker build context or Dockerfile references
echo -e "\n=== Checking for Dockerfile references ==="
rg 'Dockerfile|docker build' .github/workflows/
Length of output: 1663
🧰 Tools
🪛 yamllint
[error] 113-113: no new line character at the end of file
(new-line-at-end-of-file)
https://github.com/una-auxme/paf into 390-bug-drive-action-does-not-comment-driving-results
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
.github/workflows/build.yml (2)
116-126: Remove trailing spaces.The cache cleanup logic is correct, but there are trailing spaces on lines 121 and 127.
🧰 Tools
🪛 yamllint
[error] 121-121: trailing spaces
(trailing-spaces)
128-129: Consider more selective pruning and add newline.While pruning old images is good practice, consider:
- Being more selective by adding filters (e.g.,
--filter "label=<your-app-label>")- Adding a newline at the end of the file
- - name: Prune all images older than 1 days from self-hosted runner - run: docker image prune -a -f --filter "until=24h" + - name: Prune old images from self-hosted runner + run: docker image prune -a -f --filter "until=24h" --filter "label=org.opencontainers.image.source=${{ github.repository }}" +🧰 Tools
🪛 yamllint
[error] 129-129: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/drive.yml (3)
11-11: Document custom runner labels.The workflow uses a custom label
drivefor the self-hosted runner. While this is valid, it would be helpful to document the available custom labels in the repository.Consider adding an
actionlint.yamlconfiguration file to suppress the warning, or add a comment explaining the custom label's purpose.🧰 Tools
🪛 actionlint
11-11: label "drive" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
52-59: Add error handling for JSON processing.The JSON processing step could benefit from additional error handling:
- Check if the file exists before reading
- Handle JSON parsing errors
- Validate required fields in the parsed data
Consider updating the script:
script: | let fs = require('fs'); - let data = JSON.parse(fs.readFileSync('./artifacts/artifacts.json')); + const artifactPath = './artifacts/artifacts.json'; + if (!fs.existsSync(artifactPath)) { + core.setFailed(`Artifact file not found at ${artifactPath}`); + return; + } + try { + let data = JSON.parse(fs.readFileSync(artifactPath)); + if (!data.hasOwnProperty('is_pr')) { + core.setFailed('Required field "is_pr" missing in artifact JSON'); + return; + } + return data; + } catch (error) { + core.setFailed(`Failed to parse artifact JSON: ${error.message}`); + return; + }
113-113: Add missing newline at end of file.Add a newline character at the end of the file to comply with POSIX standards and fix the yamllint error.
run: docker image prune -a --filter "until=24h" +🧰 Tools
🪛 yamllint
[error] 113-113: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/build.yml(4 hunks).github/workflows/drive.yml(6 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build.yml
18-18: label "build" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
93-93: shellcheck reported issue in this script: SC2016:info:2:8: Expressions don't expand in single quotes, use double quotes for that
(shellcheck)
.github/workflows/drive.yml
11-11: label "drive" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
94-94: shellcheck reported issue in this script: SC2028:info:1:6: echo may not expand escape sequences. Use printf
(shellcheck)
🪛 yamllint
.github/workflows/build.yml
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 127-127: trailing spaces
(trailing-spaces)
[error] 129-129: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/drive.yml
[error] 113-113: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (6)
.github/workflows/build.yml (3)
38-49: LGTM! Well-structured Docker layer caching implementation.
The cache configuration follows best practices:
- Uses the latest stable version of actions/cache
- Separates test and latest build caches
- Implements proper cache key strategy with fallback
65-66: LGTM! Consistent cache path configuration.
The build steps correctly use absolute paths that match the cache configuration.
Also applies to: 82-83
89-113: LGTM with existing review comment.
The artifact handling logic is correct, but please address the variable expansion issue noted in the previous review comment.
🧰 Tools
🪛 actionlint
93-93: shellcheck reported issue in this script: SC2016:info:2:8: Expressions don't expand in single quotes, use double quotes for that
(shellcheck)
.github/workflows/drive.yml (3)
Line range hint 23-46: LGTM! Improved artifact handling.
The changes improve error handling and align with the build workflow's artifact naming convention.
Line range hint 72-91: LGTM! Well-structured results formatting.
The table formatting provides a clear and consistent presentation of simulation results.
112-113: Verify impact of aggressive image pruning.
The image pruning interval has been significantly reduced from 30 days to 1 day. While this helps manage disk space more aggressively, please verify that this won't affect any dependent workflows that might need older images.
✅ Verification successful
Let me check the specific docker pull commands in the workflow file to understand the image dependencies better.
Let me check the docker-compose file to understand the image configuration and dependencies.
Let me search for the docker-compose file as it might be in a different location.
Let me check if there are any other workflow files that might be affected by the image pruning.
Aggressive image pruning is safe to implement
The 1-day pruning policy is already consistently implemented across both workflow files (drive.yml and build.yml). The workflows use docker-compose for running containers and don't show any dependencies on cached images older than 24 hours. The images are pulled fresh during workflow execution with --quiet-pull, making the aggressive pruning safe for the current workflow setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for workflow dependencies on docker images
rg -l "docker.*pull" .github/workflows/
Length of output: 69
Script:
#!/bin/bash
# Check docker pull commands and their context in the workflow
rg -B 5 -A 5 "docker.*pull" .github/workflows/drive.yml
Length of output: 516
Script:
#!/bin/bash
# Check docker-compose configuration
cat docker-compose.yml
Length of output: 75
Script:
#!/bin/bash
# Find docker-compose file
fd -t f "docker-compose.y[a]?ml"
Length of output: 34
Script:
#!/bin/bash
# Check for any docker image usage in workflows
rg -B 5 -A 5 "docker.*image|FROM\s+\w+" .github/workflows/
Length of output: 1003
🧰 Tools
🪛 yamllint
[error] 113-113: no new line character at the end of file
(new-line-at-end-of-file)
| @@ -15,7 +15,7 @@ env: | |||
| jobs: | |||
| build-and-push-image: | |||
| name: Build and push image | |||
| runs-on: self-hosted | |||
| runs-on: [self-hosted, build] | |||
There was a problem hiding this comment.
💡 Codebase verification
Missing documentation for self-hosted runner configuration
The build label is only used in the build.yml workflow file and there is no documentation explaining the runner setup, required labels, or infrastructure configuration. This poses a maintainability risk as other developers won't know how to set up runners with the correct labels.
- Add documentation about self-hosted runner setup and required labels
- Consider creating an actionlint.yaml to suppress warnings for custom labels
- Document the purpose of the
buildlabel and any specific requirements for runners using this label
🔗 Analysis chain
Document custom runner label configuration.
The build label appears to be a custom label for self-hosted runners. While this is a valid practice, it should be properly documented.
Run this script to check for runner setup documentation:
Consider:
- Adding documentation about the required runner labels
- Creating an
actionlint.yamlto configure custom labels and suppress warnings
Let me gather more information about the runner configuration by checking other workflow files and documentation.
Let me check if there are any infrastructure-related files or documentation in other common locations that might contain runner setup information.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for runner setup documentation
fd -t f "README|CONTRIBUTING|SETUP|INSTALL" -X rg -i "runner|build.*label"
Length of output: 175
Script:
#!/bin/bash
# Check all workflow files for runner labels and configurations
fd -e yml -e yaml . .github/workflows -X rg -i "runs-on.*build"
# Look for any runner setup or infrastructure documentation
fd -t f -e md . .github/docs .github -X rg -i "self.hosted.*runner|runner.*setup|runner.*label"
# Check if there's an actionlint configuration
fd "actionlint" -X cat {}
Length of output: 312
Script:
#!/bin/bash
# Check for infrastructure or deployment related files
fd -t f "infrastructure|deploy|runner|setup" -X rg -i "self.hosted.*runner|runner.*setup|runner.*label"
# Check root level documentation
fd -t f -d 1 "README|CONTRIBUTING|SETUP|INSTALL" -X rg -i "github.*action|runner|workflow"
# Check if build label is used in any shell scripts or configuration files
fd -e sh -e conf -e yaml -e yml -X rg -i "build.*label|runner.*label"
Length of output: 270
🧰 Tools
🪛 actionlint
18-18: label "build" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
Description
This PR changes the logging of the simulation results. They are now always printed inside the log of the
driveaction and only put in the comments of a PR.Fixes #390
Type of change
Does this PR introduce a breaking change?
No.
Most important changes
Simulation results are now always printed in the action log.
Checklist:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes