[INFRA] Fix backport flow in merge_spark_pr.py: multi-branch, confirmation, and correct default branch#56958
Draft
zhengruifeng wants to merge 1 commit into
Draft
Conversation
…: multi-branch, confirmation, and correct default ### What changes were proposed in this pull request? Fixes three problems in the already-merged (backport) path of `dev/merge_spark_pr.py`: 1. It could only cherry-pick into a single branch, then exited. It now loops like the normal-merge path so one run can fan out to several branches. 2. It jumped straight into a cherry-pick with no per-branch confirmation. It now asks "Would you like to pick ... into another branch? (y/N)" before each pick, matching the normal-merge path. 3. The default target was always the highest-ranked branch (e.g. branch-4.x) even when the commit already landed there. It now reads the PR's "Merge Summary" comments (the ground truth this script posts) to skip branches that already contain the commit, and warns if such a branch is typed explicitly. Additionally, both the backport and normal-merge paths now compute the default pick target via a new `default_pick_branch` helper that clamps the default to a branch ranking strictly below the PR's target branch (backports flow downward). This stops a PR merged into, e.g., branch-4.1 from defaulting to branch-4.x. The merge-summary comment parser matches on the stable "merge_spark_pr.py" attribution substring, so it tolerates both the current "**Merge Summary:** ... *Posted by ...*" layout and the earlier "**Merge summary** (posted by ...)" one. ### Why are the changes needed? Backporting an already-merged PR to older maintenance branches was error-prone: no way to target multiple branches in one run, no confirmation before merging, and a default branch that was frequently wrong (suggesting a branch that already had the commit, or a forward-port up the tree). ### Does this PR introduce _any_ user-facing change? No. This is a committer tooling change only. ### How was this patch tested? Added doctests for `default_pick_branch`; existing doctests pass. Verified the merge-summary comment parser against both the current and previous comment formats, and simulated the default-branch selection across master / branch-M.x / branch-M.N targets. Generated-by: Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This is a follow-up that fixes the already-merged (backport) path of
dev/merge_spark_pr.py, which is used to cherry-pick an already-merged PR onto older maintenance branches.Three problems are fixed:
cherry_pickonce and then exited, so a backport could only target one branch per run. It now loops like the normal-merge path, so one run can fan out to several branches.Would you like to pick ... into another branch? (y/N)before each pick, matching the normal-merge path.branch-4.x) even when the commit had already landed there. It now reads the PR's "Merge Summary" comments (the ground truth this script itself posts) to skip branches that already contain the commit, prints what was already merged, and warns if such a branch is typed explicitly.Additionally, both the backport and normal-merge paths now compute the default pick target via a new
default_pick_branchhelper that clamps the default to a branch ranking strictly below the PR's target branch (backports flow downwardmaster -> branch-M.x -> branch-M.N). This stops a PR merged into, e.g.,branch-4.1from defaulting tobranch-4.x(a forward-port up the tree).The merge-summary comment parser matches on the stable
merge_spark_pr.pyattribution substring, so it tolerates both the current**Merge Summary:** ... *Posted by ...*layout and the earlier**Merge summary** (posted by ...)one.Why are the changes needed?
Backporting an already-merged PR to older maintenance branches was error-prone: no way to target multiple branches in one run, no confirmation before merging, and a default branch that was frequently wrong (suggesting a branch that already had the commit, or a forward-port up the tree).
Does this PR introduce any user-facing change?
No. This is a committer tooling change only.
How was this patch tested?
Added doctests for
default_pick_branch; existing doctests pass (python -m doctest dev/merge_spark_pr.py). Verified the merge-summary comment parser against both the current and previous comment formats, and simulated the default-branch selection acrossmaster/branch-M.x/branch-M.Ntargets.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code