sarutak commented on PR #59708: URL: https://github.com/apache/spark/pull/59708#issuecomment-6512272900
Thanks, @dongjoon-hyun. Pushed 8523320. **28, 29**: You're right; I missed that the merge and a later revert of the same subject disagree. `prompt_for_components()` now rejects `[MINOR]` and `[TRIVIAL]` with a message that a PR without a JIRA ticket puts them at the start of its title, and asks again, as it already does for a list without a primary component. So an id next to a marker in a subject now only comes from a subject that predates the rule or was not merged with this script, and the comment in `update_jira_for_pr` says that. With no marker left for it to bring back, I removed `tags |= set(title_components)` and its doctest, so the skip check reads only the title and agrees with a later revert of the same subject. That needs every prompted tag to be readable by the bracket scan: a token like `SPARK CORE` was passed through with only an unknown-tag warning, and since `main()` moves `[FOLLOWUP]` to the end, `[SPARK-1234][SQL][SPARK CORE][FOLLOWUP] Fix` would hide the `[FOLLOWUP]` from the scan. So the prompt now also rejects such tokens, and SPARK ids, which became an extra leading id when entered first. On the backport run you mention: the markers can no longer come from the prompt, so both runs read them from the same GitHub title. Other prompted components, including `[FOLLOWUP]`, can still differ between the two runs, as on master; each run's commit and its later revert agree. A new `prompt_for_components` doctest feeds `sql,minor`, ` Trivial `, `sql, spark core`, `sql,[tests]`, `sql,spark-2`, `sql][minor`, and `tests` before `sql, follow-up`, and checks that the others are rejected and the last returns `['SQL', 'FOLLOWUP']`. The direct-merge skip loop now also has `[SPARK-1234][SQL][FOLLOWUP] Fix`. The description has a paragraph for this. **30**: The doctest now says that wrapped tags follow the direct-merge policy for `FOLLOWUP`, and for `MINOR` and `TRIVIAL` when the title has no leading id. The paragraph after it now describes only older subjects. **31**: On a miss, `Component.normalize()` now returns `token.strip().upper()`, and the doctest uses ` follow-up ` and ` bogus `. 335 doctests pass, and ruff 0.14.8 `check` / `format --check` pass. All 34 mutants were killed, including accepting both markers at the prompt, rejecting only `[MINOR]` there, accepting an unreadable or partly readable token or a SPARK id there, not skipping a direct `[FOLLOWUP]` merge, checking the markers only when no primary component is given, and returning the raw token from `Component.normalize()` on a miss. -- 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]
