chore(ci): harden supply-chain workflows#134
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a Dependabot configuration for npm and GitHub Actions, alongside a new bash script, wait-for-workflow.sh, designed to poll and wait for specific GitHub workflow runs. Feedback focused on optimizing the bash script for performance, specifically suggesting a C-style for loop instead of seq and consolidating multiple jq calls into a single execution to reduce process overhead.
|
|
||
| echo "Waiting for ${workflow_file} on ${branch_name}@${commit_sha}" | ||
|
|
||
| for attempt in $(seq 1 "${max_attempts}"); do |
There was a problem hiding this comment.
Using $(seq ...) in a for loop can be inefficient as it generates all numbers at once in memory and involves a command substitution. For better performance and to follow shell scripting best practices, it's preferable to use a C-style for loop, which avoids creating a sub-process.
| for attempt in $(seq 1 "${max_attempts}"); do | |
| for (( attempt = 1; attempt <= max_attempts; attempt++ )); do |
| run_id="$(jq -r '.databaseId' <<<"${run_json}")" | ||
| run_status="$(jq -r '.status' <<<"${run_json}")" | ||
| run_conclusion="$(jq -r '.conclusion // empty' <<<"${run_json}")" |
There was a problem hiding this comment.
Calling jq multiple times to parse the same JSON object is inefficient due to the overhead of starting a new process for each call. You can achieve the same result more efficiently by using a single jq command to extract all necessary values at once.
| run_id="$(jq -r '.databaseId' <<<"${run_json}")" | |
| run_status="$(jq -r '.status' <<<"${run_json}")" | |
| run_conclusion="$(jq -r '.conclusion // empty' <<<"${run_json}")" | |
| { | |
| read -r run_id | |
| read -r run_status | |
| read -r run_conclusion | |
| } < <(jq -r '.databaseId, .status, .conclusion // empty' <<<"${run_json}") |
Summary
Verification