sarutak commented on code in PR #60031:
URL: https://github.com/apache/spark/pull/60031#discussion_r4471188385
##########
dev/merge_spark_pr.py:
##########
@@ -1994,6 +1994,8 @@ def update_jira_for_pr(pr_num, title, merge_branches,
title_components, is_rever
# initialize_jira() fails fast on a missing library/credentials when JIRA
is live.
print()
continue_maybe("Would you like to update an associated JIRA?")
+ # A revert of the innermost change, decided by all the wrappers (see
classify_wrapped_title).
+ _, is_revert = classify_wrapped_title(title)
Review Comment:
Update: the summary line and the warning change moved to SPARK-60883 with
[20], so in this PR the direction is again shown only after the push, except in
the warning of [12].
##########
dev/merge_spark_pr.py:
##########
@@ -1208,6 +1209,13 @@ def format_jira_verification(
a note instead of a warning so the expected divergence isn't flagged as a
likely wrong ticket. The Resolved/Closed warning still applies.
+ ``is_revert`` drops the Resolved/Closed warning for a ticket resolved as
Fixed, since
+ the change a revert undoes normally fixed its ticket. Any other resolution
is still
+ warned about, because the revert path cannot update such a ticket (see
+ :func:`get_jira_issue`). A revert of a revert re-lands the change and is
not a revert
+ here (see :func:`_is_revert_title`), so it keeps the warning: it usually
targets the
+ ticket that the revert reopened, so a ticket that is still resolved is
worth a look.
Review Comment:
Update: this moved to SPARK-60883 with [20]; this PR no longer changes
`format_jira_verification` or `check_linked_jira_issues`.
##########
dev/merge_spark_pr.py:
##########
@@ -1960,7 +2008,57 @@ def resolve_jira_issues(jira_ids, merge_branches,
comment, title_components=()):
)
-def update_jira_for_pr(pr_num, title, merge_branches, title_components,
is_revert=False):
+def jira_skip_tags(title):
+ """The title tags that make :func:`update_jira_for_pr` skip the JIRA
update.
+
+ >>> jira_skip_tags('Revert "[MINOR][SQL] Fix"')
+ ['MINOR']
+ >>> jira_skip_tags('[SPARK-1234][SQL][FOLLOWUP] Fix')
+ ['FOLLOWUP']
+ >>> jira_skip_tags('[SPARK-1234][SQL] Fix')
+ []
+ """
+ skip_jira_title_tags = ("MINOR", "TRIVIAL", "FOLLOWUP")
+ # Revert/Reapply PRs keep their title as is, so the tags are read inside
the wrappers,
+ # without Title.parse, which rejects some older titles such as
[SPARK-N][FOLLOWUP][MINOR].
+ tags = leading_title_tags(title)
+ if leading_jira_ids(title):
+ # Title.parse rejects an id next to [MINOR] or [TRIVIAL], and
prompt_for_components()
+ # rejects both markers, so only a subject that predates the rule or
was not merged with
+ # this script has both. Such a commit was normally merged as the fix
for that id, so
+ # the id is updated.
+ tags -= _LEADING_TAGS
+ return [tag for tag in skip_jira_title_tags if tag in tags]
+
+
+def describe_jira_action(title, is_revert):
+ """Say what :func:`update_jira_for_pr` will do, for the summary shown
before a push.
+
+ >>> describe_jira_action('[SPARK-1234][SQL] Fix', False)
+ 'resolve SPARK-1234 (or add fix versions if already resolved as Fixed)'
+ >>> describe_jira_action('Revert "[SPARK-1234][SQL] Fix"', True)
+ 'revert SPARK-1234 (remove fix versions; offer to reopen if resolved)'
+ >>> describe_jira_action('Revert "Fix"', True)
+ 'revert a ticket entered at the prompt (remove fix versions; offer to
reopen if resolved)'
+ >>> describe_jira_action('Revert "[MINOR][SQL] Fix"', True)
+ 'skip (title has [MINOR])'
+ >>> from unittest.mock import patch
+ >>> with patch.dict(describe_jira_action.__globals__, jira_disabled=True):
+ ... describe_jira_action('[SPARK-1234][SQL] Fix', False)
+ 'skip (JIRA is unreachable)'
+ """
+ if jira_disabled:
+ return "skip (JIRA is unreachable)"
+ skipped = jira_skip_tags(title)
+ if skipped:
+ return "skip (title has %s)" % ", ".join("[%s]" % tag for tag in
skipped)
+ ids = ", ".join(leading_jira_ids(title)) or "a ticket entered at the
prompt"
+ if is_revert:
+ return "revert %s (remove fix versions; offer to reopen if resolved)"
% ids
+ return "resolve %s (or add fix versions if already resolved as Fixed)" %
ids
Review Comment:
Update: the `jira` summary line moved to SPARK-60883 with [20], so this PR
no longer has these strings.
##########
dev/merge_spark_pr.py:
##########
@@ -1960,7 +2008,57 @@ def resolve_jira_issues(jira_ids, merge_branches,
comment, title_components=()):
)
-def update_jira_for_pr(pr_num, title, merge_branches, title_components,
is_revert=False):
+def jira_skip_tags(title):
+ """The title tags that make :func:`update_jira_for_pr` skip the JIRA
update.
+
+ >>> jira_skip_tags('Revert "[MINOR][SQL] Fix"')
+ ['MINOR']
+ >>> jira_skip_tags('[SPARK-1234][SQL][FOLLOWUP] Fix')
+ ['FOLLOWUP']
+ >>> jira_skip_tags('[SPARK-1234][SQL] Fix')
+ []
+ """
+ skip_jira_title_tags = ("MINOR", "TRIVIAL", "FOLLOWUP")
+ # Revert/Reapply PRs keep their title as is, so the tags are read inside
the wrappers,
+ # without Title.parse, which rejects some older titles such as
[SPARK-N][FOLLOWUP][MINOR].
+ tags = leading_title_tags(title)
+ if leading_jira_ids(title):
+ # Title.parse rejects an id next to [MINOR] or [TRIVIAL], and
prompt_for_components()
+ # rejects both markers, so only a subject that predates the rule or
was not merged with
+ # this script has both. Such a commit was normally merged as the fix
for that id, so
+ # the id is updated.
+ tags -= _LEADING_TAGS
+ return [tag for tag in skip_jira_title_tags if tag in tags]
+
+
+def describe_jira_action(title, is_revert):
Review Comment:
Update: `jira_action()` and `describe_jira_action()` moved to SPARK-60883
with [20]; the skip logic is inline in `update_jira_for_pr` again, as on master.
--
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]
