Fix unbound variable in upstream-sync.sh when SKIP_PRS is empty#709
Fix unbound variable in upstream-sync.sh when SKIP_PRS is empty#709jzding wants to merge 1 commit into
Conversation
Under set -u, empty SKIP_PRS/SKIP_COMMITS caused "unbound variable" error when iterating the arrays. Initialize arrays empty and guard iteration with length checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jack Ding <jackding@gmail.com>
WalkthroughIn Changesfilter_skipped empty-list guards
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jzding The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/upstream-sync.sh`:
- Around line 281-287: Add a regression test covering the unset/empty skip-list
path in filter_skipped, since the current shell logic in upstream-sync.sh now
relies on SKIP_PRS and SKIP_COMMITS being safely handled under set -u. Update
the existing test suite around filter_skipped to invoke it with both variables
unset and explicitly empty, and assert it does not crash or misbehave. Use the
filter_skipped and skip_pr_list/skip_commit_list flow as the reference points so
the coverage stays aligned if the shell code shifts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3ee537c8-17d8-4ebc-81a7-eacd6242306b
📒 Files selected for processing (1)
hack/upstream-sync.sh
|
@jzding: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
Fix
upstream-sync.shcrashing withunbound variableerror whenSKIP_PRSandSKIP_COMMITSenv vars are not set.Problem
Under
set -u,IFS=',' read -ra skip_pr_list <<< ""creates an array that fails when iterated with"${arr[@]}":Fix
=()read -rawhen the env var is non-empty${#arr[@]} -gt 0check🤖 Generated with Claude Code
Summary by CodeRabbit