Skip to content

[SPARK-57795][INFRA][FOLLOWUP] Refine the merge summary comment in merge_spark_pr.py#56925

Closed
zhengruifeng wants to merge 2 commits into
apache:masterfrom
zhengruifeng:reapply-merge-summary-dev6
Closed

[SPARK-57795][INFRA][FOLLOWUP] Refine the merge summary comment in merge_spark_pr.py#56925
zhengruifeng wants to merge 2 commits into
apache:masterfrom
zhengruifeng:reapply-merge-summary-dev6

Conversation

@zhengruifeng

@zhengruifeng zhengruifeng commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This reapplies the merge summary comment refinement that was merged as 1ea6ed10d75 and then reverted by eb3d853db89 (the merge was unintentional), and adds one behavioral fix on top.

Reapplied change (refines the merge summary comment posted by merge_spark_pr.py):

  1. Rename the header from **Merge summary** (posted by merge_spark_pr.py): to **Merge Summary:**.
  2. Use the full commit SHA in the commit links (previously truncated to 8 characters). The [:8] truncation is dropped in merge_pr and _do_cherry_pick, with .strip() added since run_cmd does not strip the trailing newline.
  3. Move the merge_spark_pr.py attribution to a trailing italic line: *Posted by merge_spark_pr.py*.

Additional fix (second commit):

  1. Always post the merge summary even if a backport is cancelled. Once the PR is merged into the target branch and pushed, the summary should record that landing regardless of what happens to subsequent cherry-picks. Previously, aborting/cancelling a backport (e.g. answering n at a cherry-pick conflict) raised through fail() / sys.exit() before post_merge_comment ran, so the already-successful target-branch merge was never recorded. The backport loop is now wrapped in try/finally so post_merge_comment always runs with whatever landed (at minimum the target-branch merge).

Example rendered comment:

Merge Summary:

Posted by merge_spark_pr.py

Why are the changes needed?

A cleaner, more readable merge summary comment, and a correct one: cancelling a backport must not drop the record of the merge that already landed on the target branch.

Does this PR introduce any user-facing change?

No. This only affects the developer merge tooling. The git cherry-pick -x "cherry picked from commit ..." line is unaffected (git always records the full SHA regardless of the abbreviation passed).

How was this patch tested?

Manually verified the generated comment body and confirmed the full SHA is carried through. Existing lint (ruff) passes.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (model: claude-opus-4-8)

…a backport is cancelled

Once the PR is merged into the target branch and pushed, the merge summary
comment should record that landing regardless of what happens to subsequent
cherry-picks. Previously, aborting or cancelling a backport (e.g. answering
"n" at a cherry-pick conflict) raised through fail()/sys.exit() before
post_merge_comment ran, so the already-successful target-branch merge was
never recorded.

Wrap the backport loop in try/finally so post_merge_comment always runs with
whatever landed (at minimum the target-branch merge).
@zhengruifeng zhengruifeng marked this pull request as ready for review July 2, 2026 01:45
@zhengruifeng

zhengruifeng commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

use this script to merge #56949,
and abort the 4.1 backport by ctrl+c, works as expected.

@zhengruifeng

Copy link
Copy Markdown
Contributor Author

Merge summary (posted by merge_spark_pr.py):

@zhengruifeng zhengruifeng deleted the reapply-merge-summary-dev6 branch July 2, 2026 09:48

@LuciferYang LuciferYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

late LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants