cloud-fan commented on code in PR #56058:
URL: https://github.com/apache/spark/pull/56058#discussion_r3289831954
##########
dev/merge_spark_pr.py:
##########
@@ -506,6 +503,84 @@ def cherry_pick(pr_num, merge_hash, default_branch):
return pick_ref
+def _upstream_first_sibling(target_ref, pick_ref, branch_names,
already_picked):
+ """Return the sibling branch-M.x if Upstream-First should prompt, else
None.
+
+ The policy only applies when the PR was merged into master: that's the
only case
+ where the committer can type branch-M.N at the cherry-pick prompt and
bypass the
+ rolling branch-M.x. When the PR was opened against branch-M.x the merge
itself
+ lands there (nothing to bypass), and when it was opened against branch-M.N
the
+ author already chose per-branch scope.
+
+ >>> _upstream_first_sibling("master", "branch-4.2", ["branch-4.x",
"branch-4.2"], ())
+ 'branch-4.x'
+ >>> _upstream_first_sibling("master", "branch-4.2", ["branch-4.x",
"branch-4.2"],
+ ... ("branch-4.x",))
+ >>> _upstream_first_sibling("master", "branch-4.x", ["branch-4.x"], ())
+ >>> _upstream_first_sibling("master", "branch-4.99", ["branch-4.2"], ())
+ >>> _upstream_first_sibling("branch-4.x", "branch-4.2", ["branch-4.x",
"branch-4.2"], ())
+ >>> _upstream_first_sibling("branch-4.2", "branch-3.5", ["branch-4.x",
"branch-3.5"], ())
+ """
+ if target_ref != "master":
+ return None
+ m = re.match(r"^branch-(\d+)\.(\d+)$", pick_ref)
+ if not m:
+ return None
+ candidate = "branch-%s.x" % m.group(1)
+ if candidate in branch_names and candidate not in already_picked:
+ return candidate
+ return None
+
+
+def cherry_pick(pr_num, merge_hash, default_branch, branch_names, target_ref,
already_picked=()):
+ """Prompt for a target branch and cherry-pick `merge_hash` onto it.
+
+ Enforces the Upstream-First policy (see header comment) via
+ `_upstream_first_sibling`: when the PR was merged into master and the
committer
+ types a branch-M.N target while branch-M.x is also a known release branch
AND
+ has not already received this commit, prompt to confirm whether to pick
into
+ BOTH (the policy-compliant default) or branch-M.N only (treated as a
Review Comment:
nit: "has not already received this commit" reads as if it's checking git
state, but the helper actually checks the `already_picked` set passed by the
caller — i.e., refs picked in this script invocation. If a committer ran the
script earlier (or it was reinvoked after a partial pick) and `branch-4.x`
already has the commit on the remote, the policy will still prompt to pick
both. Worth tightening to something like "has not already been picked in this
script run" so the contract matches what the code checks.
--
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]
