fix: avoid associative arrays in authorize-partner-secret.sh for bash 3.2#33
fix: avoid associative arrays in authorize-partner-secret.sh for bash 3.2#33Snakefin wants to merge 2 commits into
Conversation
… 3.2 macOS ships bash 3.2 (last GPLv2 release), which lacks declare -A. Under set -u, the [nl]=... array keys were arithmetic-evaluated as unset variables, failing with "nl: unbound variable" before the script could run. Swap the associative array for a case statement so the script works on macOS's default bash as well as bash 4+ CI runners. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe authorization script now resolves market codes to repositories and checks GitHub CLI authentication before requesting the partner API key. README guidance documents the preflight check, market mapping, and early-exit behavior. ChangesPartner secret authorization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Two real hiccups came up authorizing partners on staging: macOS's bash 3.2 (fixed in the parent commit) and a stale/expired gh session that only surfaced as a 401 at the very last step, after already going through the staging login flow. Check gh auth status up front instead and fail fast with the fix steps, and document checking it before starting in the README. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
declare -Aassociative arraysset -u, the[nl]=...array keys were arithmetic-evaluated as unset variables, causingline 21: nl: unbound variablebefore the script could even runcasestatement for the market → repo lookup — same behavior, portable across bash 3.2 (macOS default) and bash 4+ (CI runners)Test plan
bash -n scripts/authorize-partner-secret.sh— syntax OKnl→silverfin/nl_marketand proceeds past the previous failure point