sarutak commented on PR #59708: URL: https://github.com/apache/spark/pull/59708#issuecomment-6371016412
Thanks, @dongjoon-hyun. Pushed 205a17f. And thanks for catching item 9: my previous reply was wrong on both counts. The `re.findall` sentence was in the `leading_jira_ids` docstring, not in `Title.jira_ids`, and I had left it there; and two of the `Title.jira_ids` doctests still repeated `leading_jira_ids` inputs even though I said the overlapping examples were dropped. **10, 11, 16**: `leading_jira_ids` is now a single path: unwrap, then the run of leading `[SPARK-N]` brackets, or a bare leading `SPARK-N` when the title opens without a bracket. `Title.jira_ids` and the `try`/`except` are gone, and I folded the bracket scan into `leading_jira_ids` instead of keeping `_leading_spark_brackets`, since it is the only caller. So `Revert "SPARK-49004"` yields `SPARK-49004` again, and `Revert " [SPARK-1][SQL][MINOR] Fix"` yields `SPARK-1`. Only the first bare id is used (`SPARK-9545, SPARK-9547: ...` yields `SPARK-9545`); no wrapped subject in master's history has more than one bare id. A title whose id follows another tag still yields `[]`, and the docstring now says so. I re-ran the comparison on master at 67b62cb4081 (50,656 subjects from `git log --format=%s`). For every title that `Title.parse` accepts, the new path matches the parsed leading ids (0 differences). Of the 10 wrapped subjects you found losing their only id at 4c11086, 9 now get it back. The remaining one is `Revert "[WIP] [SPARK-3996]: Shade Jetty in Spark deliverables"` (2014), and `main()` rejects `[WIP]` titles before reaching the JIRA code anyway. Compared with 4c11086, the only other changes are subjects that open with a bare id and now yield it; nothing loses an id. **12**: Added `Reapply "[MINOR][SQL] Fix"` and `Reapply "[SPARK-1234][SQL][FOLLOWUP] Fix"`, called without `is_revert`, asserting that neither `resolve_jira_issues` nor `continue_maybe` is called. Also added a `Revert "SPARK-49004"` case asserting that the id reaches `revert_jira_issue`. **13**: Updated the PR description to cover all rounds, including the new skip for `Reapply` and nested wrapped titles. **14**: Dropped the `re.findall` sentence; the repeated examples went away with `Title.jira_ids`, and I also dropped the `leading_jira_ids` examples that only repeated `_unwrap_title` inputs. **15**: `update_jira_for_pr` now passes `jira_title` to `leading_jira_ids`. **17**: `Title.parse` and `leading_jira_ids` now share a small generator, `_iter_bracket_tags`, that yields each leading tag and the position after it and the spaces that follow. **18**: `_unwrap_title` is now a `_WRAPPER_RE.fullmatch` loop. The degenerate `Revert "` now comes back unchanged, and a doctest records that. While here, I also made a few edge cases consistent: the bare id accepts the same ASCII digits and case as the bracket path (`[0-9]`, case-insensitive, upper-cased), `_WRAPPER_RE` uses `re.DOTALL` so a newline in the title does not stop the unwrap, as with the old slicing, and `_unwrap_title` strips whitespace at every layer, so `Revert " Revert "[SPARK-1][SQL] Fix""` also yields its id. All 244 doctests pass, and `ruff check` / `ruff format --check` (0.14.8) are clean. -- 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]
