Copilot commented on code in PR #14526:
URL: https://github.com/apache/cloudstack/pull/14526#discussion_r4411095016
##########
.github/scripts/post-compile.sh:
##########
@@ -39,12 +39,20 @@ cd "$(git rev-parse --show-toplevel)"
export NEWVAL='${{ needs.pick_copilot_token.outputs.name != '"'"''"'"' &&
secrets[format('"'"'COPILOT_GITHUB_TOKEN_{0}'"'"',
needs.pick_copilot_token.outputs.name)] || secrets.COPILOT_GITHUB_TOKEN }}'
FILES=(
- ".github/workflows/weekly-repo-status.lock.yml"
".github/workflows/daily-issue-triage.lock.yml"
)
Review Comment:
This change drops `.github/workflows/weekly-repo-status.lock.yml` from the
post-compile patch list. If the weekly workflow still relies on the same token
rotation / redact patching, it may regress. If the intent is to scope fixes
strictly to the daily triage workflow, consider documenting why the weekly
workflow no longer needs this script (or keep it in `FILES`).
##########
.github/workflows/daily-issue-triage.md:
##########
@@ -73,16 +73,17 @@ jobs:
[ -n "$1" ] && [ "$(curl -s -o /dev/null -w '%{http_code}' \
-H "Authorization: Bearer $1" https://api.github.com/user ||
echo 000)" = "200" ]
}
- for pair in "$NAME_0|$CAND_0" "$NAME_1|$CAND_1" "$NAME_2|$CAND_2"; do
- nm="${pair%%|*}"; tok="${pair#*|}"
- if [ -z "$tok" ]; then continue; fi
- echo "::add-mask::$tok"
- if live "$tok"; then
- echo "name=$nm" >> "$GITHUB_OUTPUT"
- echo "Selected rotated token '$nm'"
- exit 0
- fi
- done
+ # name and token are passed as separate args so the name never
derives from the token
+ pick() {
+ [ -n "$2" ] || return 1
+ echo "::add-mask::$2"
+ live "$2" || return 1
+ echo "name=$1" >> "$GITHUB_OUTPUT"
+ echo "Selected rotated token '$1'"
Review Comment:
The `pick()` function currently relies on `echo` being the last command to
return success. For clarity and robustness, explicitly `return 0` after the
success path (and optionally `return 1` on failure paths only), so future edits
don’t accidentally change the function’s return status.
##########
.gitattributes:
##########
@@ -1 +1 @@
-.github/workflows/*.lock.yml linguist-generated=true merge=ours
+.github/workflows/*.lock.yml linguist-generated=true
Review Comment:
Removing `merge=ours` for generated `*.lock.yml` files will likely
reintroduce frequent merge conflicts (and potentially accidental hand-edits) in
files that are meant to be regenerated. If these lockfiles should remain
generated-only, consider restoring `merge=ours` (or an equivalent merge driver
strategy) to keep merges stable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
