rusackas opened a new pull request, #44538:
URL: https://github.com/apache/superset/pull/44538

   ### SUMMARY
   
   Enables GitHub's [merge 
queue](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue)
 for `master` via a new `github.rulesets` entry in `.asf.yaml`, alongside the 
existing classic `protected_branches` config (the two mechanisms coexist; 
GitHub evaluates both). Legal cleared this for ASF projects in 
[LEGAL-599](https://issues.apache.org/jira/browse/LEGAL-599) (same precedent as 
Dependabot's temporary-branch creation).
   
   With this merged, a committer clicks "Merge when ready" instead of merging 
directly; GitHub serializes approved PRs into a queue, batches them, builds 
each batch against the current tip of `master` plus whatever's ahead of it in 
the queue, and only merges once the batch's required checks pass against that 
combined state. This catches the class of bug where two individually-green PRs 
conflict once actually combined (this repo already has 
`enforce-single-migration-head` for exactly one instance of that class — two 
migration heads that each looked fine alone), and keeps `master` from ever 
landing a change that wasn't actually tested against what it's landing on top 
of.
   
   **Why the workflow changes, not just `.asf.yaml`:** GitHub only runs a 
workflow against a merge queue's synthetic ref if the workflow explicitly 
listens for the `merge_group` event — none of this repo's workflows did. 
Without adding it, every PR entering the queue would sit waiting for 
required-check statuses that simply never get reported, and time out. This PR 
adds `merge_group:` triggers to every workflow that produces a check the new 
ruleset requires, plus two small logic fixes where the existing 
pull_request/push-oriented code would otherwise break under `merge_group`:
   
   - `enforce-single-migration-head.yml` — its file-list-based skip 
optimization reads `context.issue.number`, which doesn't exist for 
`merge_group`. It now always runs the real check for that event, matching the 
file's own existing "never silently skip a required check" philosophy — and 
this is precisely the scenario the check exists for.
   - `pre-commit.yml` — its changed-files diff previously assumed `HEAD^1` 
means "tip of the base branch," true for `pull_request`'s synthetic merge 
commit but not guaranteed for `merge_group`'s. It now reads the event's own 
explicit `merge_group.base_sha` instead.
   - `scripts/change_detector.py` (backs `superset-frontend.yml`, 
`superset-e2e.yml`, and all three Python test workflows) — previously raised on 
any event type it didn't recognize, which included `merge_group`. Added a 
branch that diffs against `merge_group.base_sha` via a new shared 
`fetch_changed_files_compare()` helper (the existing `push` path now uses the 
same helper too, no behavior change there).
   
   **Two required checks are deliberately left out of the new ruleset** and 
don't get `merge_group` support — they remain required only via the existing 
classic `protected_branches` config, i.e. they still gate a PR before a 
committer can add it to the queue, they just don't re-run once it's in one:
   
   - `lint-check` validates the PR's own title. That's meaningless re-run 
against a merge-group ref.
   - `dependency-review` — `actions/dependency-review-action` has a confirmed, 
currently open, unfixed bug on `merge_group` events 
([actions/dependency-review-action#843](https://github.com/actions/dependency-review-action/issues/843)):
 it destructures `github.event.pull_request`, which doesn't exist there, and 
crashes outright.
   
   **Why the raw ruleset payload syntax, not the nicer `merge_queue` 
convenience key:** that convenience key exists but is still an open, unmerged 
PR upstream 
([apache/infrastructure-asfyaml#119](https://github.com/apache/infrastructure-asfyaml/pull/119))
 as of this writing, so using it here would fail INFRA-bot schema validation. 
`.asf.yaml` can move to the convenience syntax once that lands.
   
   **Tuning choices in the new `merge_queue` rule** (each has an inline comment 
in `.asf.yaml` too):
   - `check_response_timeout_minutes: 60` — this required-check matrix runs 
full Playwright/Cypress/multi-DB-flavor suites that can legitimately take 40+ 
minutes.
   - `grouping_strategy: ALLGREEN` — every entry in a batch must pass before 
any of them merge, so nothing ships on a check result computed against a 
different commit than the one landing.
   - `max_entries_to_build`/`max_entries_to_merge: 5` — kept modest given how 
expensive and wide this required-check matrix already is; a bigger batch 
multiplies the blast radius of one unrelated entry's failure evicting the whole 
group.
   - `merge_method: SQUASH` — matches `enabled_merge_buttons` above; squash is 
the only merge method this repo has enabled.
   - `min_entries_to_merge_wait_minutes: 5` — short enough that a lone PR isn't 
stuck waiting on nothing to batch with.
   
   ### TESTING INSTRUCTIONS
   
   - `pytest tests/unit_tests/scripts/change_detector_test.py` — 13 passed (8 
existing + 5 new: compare-helper delegation, push still delegates to it, 
merge_group happy path, merge_group fails closed without 
`MERGE_GROUP_BASE_SHA`).
   - `ruff check` / `ruff format --check` on the two touched Python files — 
clean.
   - `zizmor` (the same GHA security linter `validate-all-ghas` runs in CI) 
against every touched workflow/action file — no findings.
   - `python3 -c "import yaml; yaml.safe_load(...)"` on `.asf.yaml` and every 
touched workflow file — all parse. Also parsed `.asf.yaml` back with PyYAML to 
confirm `rulesets` landed as a sibling of `protected_branches`, not nested 
inside it, and printed the parsed ruleset structure to confirm it matches the 
exact shape GitHub's Rulesets API expects.
   - Could not run `pre-commit` itself end-to-end — its git-diffing is broken 
in my environment on an unrelated host xcrun/libxcrun architecture mismatch, 
not something this PR introduces. The checks above cover what pre-commit would 
have run for these file types.
   - Not yet verified against a live merge-group run — only checkable once this 
actually merges and a real queue entry exercises it. Worth watching the first 
few queued PRs closely after this lands.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


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