sarutak commented on PR #59708: URL: https://github.com/apache/spark/pull/59708#issuecomment-6482340705
Thanks, @dongjoon-hyun. Pushed 3731bcf and f309b6a, which address items 23-27 in this PR, and updated the PR description. **23**: `update_jira_for_pr` now reads the skip tags with a new `leading_title_tags()`, from the same leading bracket run as the ids, after unwrapping and with aliases normalized, instead of through `Title.parse`. On your question whether the `[MINOR]`/`[TRIVIAL]` subjects should keep updating their ticket: I think they should. `Title.parse` rejects an id next to those markers, so a subject has both only if it predates that rule or the committer added the marker at the component prompt. The older ones were normally merged as the fix for that id; the skip tags only came with d1a7c29232e in 2026, and no subject since then has an id and a marker. For example, SPARK-23960 got fix version 2.4.0 when 271c891b919 `[SPARK-23960][SQL][MINOR] Mark HashAggregateExec.bufVars as transient` was merged, and still has it although e904dfaf0d1 reverted that commit the same day, which is what the revert path is for. So `[FOLLOWUP]` is read from the run unconditionally, and `[MINOR]`/`[TRIVIAL]` in t he title skip the update only when it has no leading id, which is the same as at 14e286c for every title `Title.parse` accepts. A `[MINOR]` that the committer chooses at the component prompt still skips, since `title_components` is added after the markers are dropped. Replaying this over the 50,753 subjects of master at 1a2450b3e5c, the 17 wrapped subjects whose decision changes against master are all reverts of `[FOLLOW-UP]` changes, now skipped. Counted as you did, over all subjects if reverted: the 21 `[FOLLOWUP]`/`[FOLLOW-UP]` subjects with a leading id are now skipped, the 119 with `[MINOR]`/`[TRIVIAL]` and a leading id still update it, and 193 with a skip tag but no leading id (191 of them `[MINOR]`/`[TRIVIAL]` only, e.g. `[SQL][MINOR] ...`) are now skipped like a direct `[MINOR]` merge. Only one of those 193, `[Minor] [SQL] [SPARK-6729] ...` from 2015, has an id later in the run; it is now skipped rather than offering to revert SPARK-6729 as master did, and I left it so rathe r than widen the rule for one subject. The doctest covers `[SPARK-1234][SQL][FOLLOWUP][MINOR]` (skipped, `Revert` and `Reapply`), `[MINOR][TRIVIAL][SQL]` (skipped), `[SQL][MINOR]` and `[MINOR][SPARK-1234]` (skipped), and `[SPARK-1234][SQL][MINOR]` / `[SPARK-1234][SQL][TRIVIAL]` (still revert or resolve SPARK-1234). **24**: `is_followup` now comes from `leading_title_tags(title)`, so a wrapped `[FOLLOWUP]` or `[FOLLOW-UP]` title skips the similarity check like a direct one. For an ordinary title, `title` there is already normalized, so this matches `title_components`. The doctest also checks that `FOLLOWUP` in the title text does not count. **25**: The id collection, the Epic/Umbrella check, and the match confirmation moved from `main()` into `check_linked_jira_issues(pr_num, title)`, with no other change than item 24. Its doctest mocks `asf_jira` and uses the #59587 title: only `SPARK-60328` is fetched and confirmed, so `SPARK-34261` as an Epic does not block. It also covers confirming two leading ids, aborting when one is declined or the prompt is answered with Enter, a ticket that cannot be fetched (the next id is still checked), a leading Epic and Umbrella both listed before any confirmation, and the `is_followup` cases from item 24. Going back to the whole-title regex in this helper now fails 5 doctests. **26**: `resolve_jira_issues` now takes `jira_ids`, and `update_jira_for_pr` extracts them once. They stay above the skip check, since the item 23 rule needs them. The resolve-path doctest now also passes two leading ids, and a title without an id. **27**: Added `Component.normalize(token)`, used by `main()`, `prompt_for_components()`, which had a third copy, and `leading_title_tags()`, which both `update_jira_for_pr` and `check_linked_jira_issues` use. 291 doctests pass, and ruff 0.14.8 `check` / `format --check` pass. All 25 mutants were killed, including reading the skip tags only via `Title.parse` or only for wrapped titles, letting `[MINOR]` skip a title with an id, `is_followup` by raw substring or not inside wrappers, checking or resolving only the first id, not aborting on a declined match or on Enter, stopping at or not skipping an unfetchable ticket, dropping a `[MINOR]` chosen at the prompt, listing only the first blocker, the old regex in `check_linked_jira_issues` or in `update_jira_for_pr`, and an identity `Component.normalize`. The wrapper-parity follow-up now only needs to decide revert vs. reapply by nesting. -- 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]
