sunchao commented on code in PR #5885:
URL: https://github.com/apache/datafusion-comet/pull/5885#discussion_r3997726164


##########
.github/workflows/ci.yml:
##########
@@ -253,7 +256,10 @@ jobs:
   spark_3_4:
     name: Spark SQL Tests (Spark 3.4)
     needs: changes
-    # Queue-only by default; PRs need the `run-spark-3.4-tests` label.
+    # Spark 3.4 is deprecated, so this is the one test job the merge queue does
+    # not run: it needs the `run-spark-3.4-tests` label on a pull request, or a
+    # manual workflow_dispatch. It stays in `required_checks.needs` so that a
+    # labelled run still gates the pull request it was asked for.

Review Comment:
   [P2] Correct the promise that applying the label gates the PR
   
   Could we correct this guarantee here and in the contributor docs? The 
documented `gh pr edit --add-label run-spark-3.4-tests` operation fires a 
`labeled` event. On that event, `required_checks.name` deliberately publishes 
`Required Checks (label run)`, while `.asf.yaml` names only `Required Checks` 
as required. Keeping `spark_3_4` in `needs` therefore propagates a failure or 
cancellation to the non-required label status without changing an earlier green 
required verdict. This PR also removes the subsequent Spark 3.4 queue run, so 
that later check no longer catches the failure.
   
   I verified the base/head routing scripts and aggregator configuration 
locally. A subsequent push with the label retained does include Spark 3.4 in 
the normal `Required Checks` verdict. Please document that applying the label 
alone produces an advisory result, and explain the subsequent push needed for 
the required gate, or implement the gating promised by the current wording.



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