spark-test commented on code in PR #59708:
URL: https://github.com/apache/spark/pull/59708#discussion_r4352860356
##########
dev/merge_spark_pr.py:
##########
@@ -2156,6 +2175,92 @@ def __str__(self) -> str:
return parts + (" " if parts else "") + self.text
+def _unwrap_title(title: str) -> str:
+ """Strip a ``Revert "..."`` / ``Reapply "..."`` wrapper from a PR title.
+
+ ``main()`` keeps Revert and Reapply PR titles verbatim, so the JIRA id,
+ leading markers, and components all live inside the quotes. Unwrapping
here
+ lets the id-extraction and skip-tag logic see them. A title without such a
+ wrapper is returned unchanged.
+
+ >>> _unwrap_title('Revert "[SPARK-1234][SQL] Fix"')
+ '[SPARK-1234][SQL] Fix'
+ >>> _unwrap_title('Reapply "[SPARK-1234][SQL] Fix"')
+ '[SPARK-1234][SQL] Fix'
+ >>> _unwrap_title('[SPARK-1234][SQL] Fix')
+ '[SPARK-1234][SQL] Fix'
+ """
+ for prefix in ('Revert "', 'Reapply "'):
+ if title.startswith(prefix) and title.endswith('"'):
Review Comment:
**Nit (P3):** This strips only one wrapper. `main()` still treats a
revert-of-revert title such as `Revert "Revert "[SPARK-1234][SQL] Fix""` as a
revert PR (it starts with `Revert "` and ends with `"`), but after one unwrap
the inner text still begins with `Revert`, so `Title.parse` raises and
`_leading_spark_brackets` finds no bracket at position 0. I ran the helpers at
2233a6b: `leading_jira_ids` returns `[]` for this title and for `Revert
"Reapply "[SPARK-1234][SQL] Fix""`. On master, `re.findall` found `SPARK-1234`.
As a result, `main()` skips the Epic/Umbrella blocker and the JIRA-match
confirmation for these PRs, and `update_jira_for_pr` calls `revert_jira_issue`
with an empty default id (and the skip-tag check misses inner
`[MINOR]`/`[FOLLOWUP]`). Could you keep unwrapping in a loop until no `Revert
"..."`/`Reapply "..."` wrapper remains, and add a doctest for the nested case?
This extends existing discussions
[1](https://github.com/apache/spark/pull/59708#issuecomment-6343253163),
[2](https://github.com/apache/spark/pull/59708#pullrequestreview-5615544014).
**Verification:**
- **Regression:** A doctest shows that a revert-of-revert title yields the
inner leading SPARK id from leading_jira_ids, and that _unwrap_title returns
the innermost title.
- **Compatibility:** The existing doctests for single-level Revert/Reapply
titles, parse-failure fallback titles, and body-id exclusion still pass
unchanged.
--
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]
