[CALCITE-5584] Publish website action only triggers for the last commit in the chain - #5136
[CALCITE-5584] Publish website action only triggers for the last commit in the chain#5136rubenada wants to merge 2 commits into
Conversation
| git status | sed -n 's/deleted by us://p' | xargs git add | ||
| git cherry-pick --continue --no-edit | ||
|
|
||
| # github.event.commits is capped at 20 entries. Warn loudly if we hit |
There was a problem hiding this comment.
Is there some documentation around the 20 entries limit? I was looking at https://docs.github.com/en/webhooks/webhook-events-and-payloads#push and there it says that commits array contains at most 2048 entries.
There was a problem hiding this comment.
You're right! I should've double checked that. It seems that 20 was an old cap, and nowadays is 2048 (i.e. virtually no limit for our purpose). I'll remove this check and warning.
| if [ "$(echo "$COMMITS" | jq 'length')" -ge 20 ]; then | ||
| echo "::warning::Push contains 20+ commits — github.event.commits may be truncated; some commits may not be cherry-picked." | ||
| fi |
There was a problem hiding this comment.
Do you have an example of how this looks?
There was a problem hiding this comment.
This block is actually unnecessary (see comment above), so it'll go away.
| echo "Cherry-picking commit: $commit_sha" | ||
| echo "----------------------------------------" | ||
|
|
||
| if ! git cherry-pick --strategy=recursive -X theirs "$commit_sha"; then |
There was a problem hiding this comment.
The if condition is slightly different than before. Did you test the action with cherry-pick conflicts to ensure it works?
There was a problem hiding this comment.
My understanding is that in the old code, the "if" block was actually never reachable, because github runs the command transparently with -e , so if the cherry-pick failed, the action was aborted and the "if" was never reached.
IIANM, inserting the command directly on the "if" condition will achieve the intended purpose.
|
3 participants
Jira Link
CALCITE-5584
Changes Proposed
Adjust publish-non-release-website-updates.yml to be able to process up to 20 commits (limit of github.event.commits); that should be enough for Calcite, where normally only one or a few commits land together.