rusackas commented on code in PR #44538:
URL: https://github.com/apache/superset/pull/44538#discussion_r4080707818
##########
scripts/change_detector.py:
##########
@@ -151,6 +151,13 @@ def fetch_changed_files_pr(repo: str, pr_number: str) ->
List[str]:
return [file_info["filename"] for file_info in files]
+def fetch_changed_files_compare(repo: str, base_sha: str, head_sha: str) ->
List[str]:
+ """Fetches files changed between two commits using the GitHub compare
API."""
+ compare_url =
f"https://api.github.com/repos/{repo}/compare/{base_sha}...{head_sha}";
+ comparison_data = fetch_files_github_api(compare_url)
+ return [file["filename"] for file in comparison_data["files"]]
Review Comment:
The 300-file cap on the compare API is real, but the existing `len(files) >=
99` fail-safe a few lines down already treats that as "trigger everything" for
every event type, merge_group included, so an under-count here never silently
skips a check.
##########
.github/workflows/superset-e2e.yml:
##########
@@ -7,6 +7,8 @@ on:
- "[0-9].[0-9]*"
pull_request:
types: [synchronize, opened, reopened, ready_for_review]
+ merge_group:
+ types: [checks_requested]
Review Comment:
For merge_group, github.event_name isn't 'pull_request', so the ref here
falls through to github.sha -- which for a merge_group event is documented as
the queue's synthetic merge commit. This checkout already gets the queued
source.
##########
.asf.yaml:
##########
@@ -103,3 +103,92 @@ github:
required_approving_review_count: 1
required_signatures: false
+
+ # GitHub's merge queue rule type only exists on rulesets, not on the classic
+ # `protected_branches` API above -- the two mechanisms coexist (GitHub
+ # evaluates both), so this adds a merge queue on `master` without touching
+ # the existing classic config, which still gates PR-entry-into-queue
+ # readiness (review count, code-owner approval, etc).
+ #
+ # Raw payload syntax, not the newer `merge_queue` convenience key: that key
+ # is still pending in apache/infrastructure-asfyaml#119 as of this writing
+ # and isn't deployed yet, so using it here would fail INFRA-bot validation.
+ # Switch this entry to the convenience syntax once that PR merges.
+ #
+ # `required_status_checks` below intentionally omits two checks that are
+ # required in the classic config above: `lint-check` (validates the PR's
+ # own title -- meaningless once re-run against the queue's synthetic
+ # merge-group ref) and `dependency-review` (actions/dependency-review-action
+ # has a confirmed, unfixed bug against merge_group events as of this
+ # writing -- see actions/dependency-review-action#843 -- it destructures
+ # `github.event.pull_request`, which merge_group doesn't have, and crashes).
+ # Both still gate a PR before a committer can add it to the queue via the
+ # classic `required_status_checks` above; they just don't re-run once it's
+ # in the queue.
+ rulesets:
+ - name: "Merge Queue"
+ target: branch
+ enforcement: active
+ conditions:
+ ref_name:
+ include:
+ - "~DEFAULT_BRANCH"
+ exclude: []
+ rules:
+ - type: required_status_checks
+ parameters:
+ strict_required_status_checks_policy: false
+ required_status_checks:
+ - context: enforce-single-migration-head
+ integration_id: -1
+ - context: pre-commit (current)
+ integration_id: -1
+ - context: frontend-build
+ integration_id: -1
+ - context: cypress-matrix-required
+ integration_id: -1
+ - context: playwright-tests-required
+ integration_id: -1
+ - context: test-mysql
+ integration_id: -1
+ - context: test-postgres-required
+ integration_id: -1
+ - context: test-postgres-hive
+ integration_id: -1
+ - context: test-postgres-presto
+ integration_id: -1
+ - context: test-sqlite
+ integration_id: -1
+ - context: unit-tests-required
+ integration_id: -1
Review Comment:
Fair, added a sync-required comment so this doesn't drift.
--
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]
