viirya commented on PR #56058: URL: https://github.com/apache/spark/pull/56058#issuecomment-4519633781
Thanks for the review @cloud-fan! Pushed follow-up `188d6c56ab8` addressing all five comments: 1. **Gating on `target_ref == "master"`** — the previous `already_picked`-based suppression was incidental and missed the cross-`branch-M.N` cherry-pick case you flagged. Now `cherry_pick` takes `target_ref` and short-circuits when it isn't master. 2. **Removed unreachable `candidate != pick_ref` check** — regex constrains `pick_ref` to digit minor, so it can never equal `branch-M.x`. 3. **Removed redundant `clean_up()` before `fail()` in the abort branch** — `fail()` already cleans up; the explicit call caused a double "Restoring head pointer to ..." print. 4. **Aligned divider width to `"=" * 80`** to match the rest of the file. 5. **Extracted `_upstream_first_sibling` as a pure helper with doctests** covering: PR target master (positive + `already_picked` suppression), PR target branch-M.x and branch-M.N (both gated out), absent branch-M.x sibling, non-matching `pick_ref`. `python3 -m doctest dev/merge_spark_pr.py` now passes 40/40 (was 34/34). The new policy logic is now covered by doctests; the end-to-end interactive flow still requires committer access to a live PR to exercise (noted in the PR description). PTAL. -- 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]
