felipepessoto commented on PR #12781:
URL: https://github.com/apache/gluten/pull/12781#issuecomment-5349789317

   Changed in `ea6e9ef`: **dropped the PR-author restriction**, so 
`/delta-test` now gates on the command alone — the same as the repo's existing 
comment triggers, `velox_backend_ansi.yml` (`/ansi-test`, `/ansi-analyze`) and 
`take.yml`, neither of which has an author or permission check.
   
   This supersedes my earlier reasoning in [this 
thread](https://github.com/apache/gluten/pull/12781#discussion_r3788252935), 
where I argued for author-only on the grounds that maintainers already have 
`workflow_dispatch`. Consistency with the pipeline next door is the better 
default, and author-only did block a real case: a reviewer wanting the Delta 
suite run against a contributor's PR before merging.
   
   **One deliberate difference from `/ansi-test` retained:** the exact-token 
match, rather than `contains()`. `contains()` fires on a mention mid-sentence — 
this very comment thread would trigger `/ansi-test` several times over — and 
would also match a future `/delta-test-arm`. That was raised in review and is 
worth keeping at ~11 job-hours a run.
   
   So the gate is now:
   
   ```yaml
       if: >-
         github.event_name != 'issue_comment' ||
         (github.event.issue.pull_request &&
          (github.event.comment.body == '/delta-test' ||
           startsWith(github.event.comment.body, '/delta-test ') ||
           startsWith(github.event.comment.body, format('/delta-test{0}', 
fromJSON('"\r"'))) ||
           startsWith(github.event.comment.body, format('/delta-test{0}', 
fromJSON('"\n"')))))
   ```
   
   **Worth flagging for reviewers:** this promotes the three "restore but never 
save" cache guards from belt-and-braces to load bearing, since any user can now 
start a run that builds and executes the PR's code — and the ccache key is 
shared with `velox_backend_x86.yml`. Those guards were already in place and are 
unchanged; the [e2e test 
above](https://github.com/apache/gluten/pull/12781#issuecomment-5347525074) 
confirmed `Save Ccache` is skipped on comment-triggered runs.
   
   Truth table re-run, 12/12: accepted from any user (bare, with trailing text, 
and as the first line of a CRLF or LF multi-line comment); still rejecting 
`/delta-test-arm`, `/delta-testers`, a mid-sentence mention, comments on non-PR 
issues, and ordinary chatter. actionlint clean. PR description updated to match.
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to