You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zhengruifeng (via GitHub)" <gi...@apache.org> on 2026/07/01 01:53:46 UTC
[PR] [INFRA] Post a merge summary comment on the PR in merge_spark_pr.py [spark]
zhengruifeng opened a new pull request, #56917:
URL: https://github.com/apache/spark/pull/56917
### What changes were proposed in this pull request?
After a PR is merged (and any cherry-picks are done), `dev/merge_spark_pr.py` now posts a single comment on the PR recording every branch the change landed on and a link to the resulting commit, e.g.:
```
MERGE_SPARK_PR:
- merged into master https://github.com/apache/spark/commit/0e1101aa
- merged into branch-4.x https://github.com/apache/spark/commit/964f8da1
- merged into branch-4.2 https://github.com/apache/spark/commit/9a6bd1ef
```
New helpers `comment_pr` (POST `issues/{pr}/comments`) and `post_merge_comment` build and post the comment. `_do_cherry_pick` now returns `(ref, hash)` and `cherry_pick` returns a list of such pairs so the exact backport commits can be listed. The comment is posted from both the normal merge path and the already-merged backport path, and is skipped when `GITHUB_OAUTH_KEY` is unset.
### Why are the changes needed?
A merged PR shows up on GitHub as Closed, and the master merge commit is folded into the close event rather than a "referenced in commit" link, so it is not obvious from the PR page which branches actually received the change. An explicit comment makes the merge fully traceable from the PR.
### Does this PR introduce _any_ user-facing change?
No. Committer tooling only.
### How was this patch tested?
`python -m py_compile` and the module doctests pass.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-opus-4-8)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57795][INFRA] Post a merge summary comment on the PR in merge_spark_pr.py [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #56917:
URL: https://github.com/apache/spark/pull/56917#issuecomment-4853271910
merged to master
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57795][INFRA] Post a merge summary comment on the PR in merge_spark_pr.py [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #56917:
URL: https://github.com/apache/spark/pull/56917#issuecomment-4849855561
use this script to merge https://github.com/apache/spark/pull/56914, it commented https://github.com/apache/spark/pull/56914#issuecomment-4849842748
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57795][INFRA] Post a merge summary comment on the PR in merge_spark_pr.py [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #56917: [SPARK-57795][INFRA] Post a merge summary comment on the PR in merge_spark_pr.py
URL: https://github.com/apache/spark/pull/56917
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57795][INFRA] Post a merge summary comment on the PR in merge_spark_pr.py [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57795][INFRA] Post a merge summary comment on the PR in merge_spark_pr.py [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #56917:
URL: https://github.com/apache/spark/pull/56917#discussion_r3503720864
##########
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:
Good catch. Went with the plain-prose option: the header is now `**Merge summary** (posted by `merge_spark_pr.py`):` since the comment is human-facing and nothing parses it. Wiring a consumer (e.g. `pr_merge_status.py`) to read a machine-readable marker with full SHAs is left as a follow-up. Addressed in 6422b71151b.
##########
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:
Fixed in 6422b71151b -- `_do_cherry_pick` docstring now says it 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org