cloud-fan commented on code in PR #56917:
URL: https://github.com/apache/spark/pull/56917#discussion_r3503200740
##########
dev/merge_spark_pr.py:
##########
@@ -421,6 +422,42 @@ def close_pr(pr_num):
return None
+def comment_pr(pr_num, body):
+ url = "%s/issues/%s/comments" % (GITHUB_API_BASE, pr_num)
+ data = json.dumps({"body": body}).encode("utf-8")
+ request = Request(url, data=data, method="POST")
+ request.add_header("Content-Type", "application/json")
+ request.add_header("Accept", "application/vnd.github+json")
+ if GITHUB_OAUTH_KEY:
+ request.add_header("Authorization", "token %s" % GITHUB_OAUTH_KEY)
+ try:
+ return json.load(urlopen(request))
+ except HTTPError as e:
+ print_error("Failed to comment on PR #%s: HTTP %s %s" % (pr_num,
e.code, e.reason))
+ return None
+
+
+def post_merge_comment(pr_num, merged_commits):
+ """Post a comment on the PR recording every branch the change landed on
and a
+ link to the resulting commit, so the merge is traceable from the PR page.
+
+ ``merged_commits`` is an ordered list of (branch, commit_hash) pairs, the
merge
+ sink first followed by each cherry-pick target.
+ """
+ if not merged_commits:
+ return
+ lines = [
+ "- merged into %s %s/%s" % (ref, GITHUB_COMMIT_BASE, commit_hash)
+ for ref, commit_hash in merged_commits
+ ]
+ body = "MERGE SUMMARY BY MERGE_SPARK_PR:\n" + "\n".join(lines)
Review Comment:
The `MERGE SUMMARY BY MERGE_SPARK_PR:` header reads like a machine-readable
marker (all-caps, echoing the script name), but nothing actually consumes it —
`pr_merge_status.py` and the rest of `dev/` don't parse it. Is it meant to be
machine-read?
- **If not**, plain markdown reads better on the PR page, e.g. `**Merge
summary** (posted by `merge_spark_pr.py`):`.
- **If so**, it's worth making load-bearing: `pr_merge_status.py`
reconstructs landed branches by grepping the `Closes #<pr> from ` trailer
(`dev/pr_merge_status.py:260-265`), and its own docstring calls that fragile —
the exact weakness this comment is pitched to fix. Consider emitting one `<!--
merge-summary branch=<ref> commit=<full-sha> -->` line per branch (full SHA
rather than the 8-char short form), and having `pr_merge_status.py` read the PR
comments via `gh` (it already calls `gh()`), parse those when present, and fall
back to the trailer grep otherwise.
Non-blocking — just flagging the marker-vs-prose ambiguity.
##########
dev/merge_spark_pr.py:
##########
@@ -559,7 +596,7 @@ def _do_cherry_pick(pr_num, merge_hash, pick_ref):
print("Pull request #%s picked into %s!" % (pr_num, pick_ref))
print("Pick hash: %s" % pick_hash)
- return pick_ref
+ return pick_ref, pick_hash
Review Comment:
Nit: `_do_cherry_pick`'s docstring (line 571) still says "Returns the pushed
ref.", but it now returns the `(pick_ref, pick_hash)` pair. `cherry_pick`'s
docstring was updated for the new shape; this one looks like it was just
missed. Suggest: "Returns the (pushed ref, pushed commit hash) pair."
--
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]
