PR Review: Fix bash 3.2 Compatibility for macOS

Summary

This PR successfully addresses macOS compatibility issues by replacing bash 4+ features with bash 3.2-compatible alternatives. The changes are well-implemented and solve real problems.

Code Quality & Best Practices ✅

Strengths:

• Excellent bash 3.2 compatibility solution using file-based storage instead of associative arrays
• Proper cleanup with trap ensures temp files are removed even on failure
• Improved error handling (lines 276-279) now shows detailed errors when issue creation fails
• Smart delimiter change from : to | allows labels like priority:critical to work
• Better JSON handling by querying files directly avoids bash variable escaping issues

Suggestions for improvement:

1. Reduce code duplication in the create_issue() function (lines 250-264):

# Instead of duplicating the gh issue create command:
local gh_args=(--repo "$REPO" --title "$full_title" --body-file "$body_file")
[[ -n "$labels" ]] && gh_args+=(--label "$labels")
[[ -n "$milestone" && "$milestone" != "null" ]] && gh_args+=(--milestone "$milestone")
issue_url=$(gh issue create "${gh_args[@]}" 2>&1)

2. Improve grep safety in get_issue_map() (line 117) for IDs with special characters:

grep "^${1}=" "$ISSUE_MAP_FILE" 2>/dev/null | cut -d= -f2 | tail -1

Potential Issues 🐛

1. Concurrent write safety: The set_issue_map() function appends to the file, relying on tail -1 to handle duplicates. While unlikely to cause issues in sequential execution, consider making it more robust:

set_issue_map() {
    local key="$1" value="$2"
    grep -v "^${key}=" "$ISSUE_MAP_FILE" > "${ISSUE_MAP_FILE}.tmp" 2>/dev/null || true
    echo "${key}=${value}" >> "${ISSUE_MAP_FILE}.tmp"
    mv "${ISSUE_MAP_FILE}.tmp" "$ISSUE_MAP_FILE"
}

2. Empty issues array handling: Add graceful handling when issues array is empty:

if [[ $issue_count -eq 0 ]]; then
    echo -e "${YELLOW}No issues to create${NC}"
    exit 0
fi

Performance ⚡

Good:

• Querying JSON directly is more efficient than storing in bash variables
• File-based storage is performant for expected volume (<100 issues)

Security 🔒

Good practices observed:

• ✅ Proper mktemp usage
• ✅ Cleanup with trap prevents temp file leakage
• ✅ Using --body-file prevents command injection
• ✅ Proper quoting throughout

Testing 🧪

Recommendations:

1. Add ShellCheck to CI to catch common bash issues
2. Add smoke test that runs --dry-run on both bash 3.2 (macOS) and bash 4+ (Linux)
3. Document bash version requirement in script header:

# Requires: bash 3.2+, gh CLI, jq

4. Consider adding version check:

if [[ "${BASH_VERSINFO[0]}" -lt 3 ]]; then
    echo -e "${RED}Error: bash 3.0 or higher required${NC}"
    exit 1
fi

Overall Assessment ⭐

Verdict: Approve with minor suggestions

This is a solid fix that solves real compatibility issues. The core changes are well-designed:

• File-based mapping is a clean solution for bash 3.2 compatibility
• Delimiter change fixes label parsing
• Direct JSON queries avoid escaping issues
• Error handling improvements aid debugging

The suggestions above are nice-to-haves that would make the script more robust, but they're not blockers. The PR is ready to merge as-is, with optional follow-up improvements.

Score: 8.5/10

Great work! 🎉