You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/06/13 22:24:56 UTC

[arrow] branch master updated: ARROW-5083: [Developer] PR merge script improvements: set already-released Fix Version, display warning when no components set

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new d14ba3a  ARROW-5083: [Developer] PR merge script improvements: set already-released Fix Version, display warning when no components set
d14ba3a is described below

commit d14ba3ae0073451636fc01c24c189703862cf1ee
Author: Wes McKinney <we...@apache.org>
AuthorDate: Thu Jun 13 17:24:47 2019 -0500

    ARROW-5083: [Developer] PR merge script improvements: set already-released Fix Version, display warning when no components set
    
    This also resolves ARROW-5472.
    
    Author: Wes McKinney <we...@apache.org>
    
    Closes #4554 from wesm/ARROW-5083 and squashes the following commits:
    
    dbf9bb8f1 <Wes McKinney> Actually print the formatted issue summary
    9c8d34540 <Wes McKinney> test output when there are components
    a713d681c <Wes McKinney> Merge script improvements, allow setting released fix version, display warning when no components are set
---
 dev/merge_arrow_pr.py      | 101 ++++++++++++++++++++++++++++-----------------
 dev/test_merge_arrow_pr.py |  73 ++++++++++++++++++++++++++------
 2 files changed, 124 insertions(+), 50 deletions(-)

diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py
index 6413365..b295ac5 100755
--- a/dev/merge_arrow_pr.py
+++ b/dev/merge_arrow_pr.py
@@ -166,25 +166,21 @@ class JiraIssue(object):
                     default_fix_versions = [x for x in default_fix_versions
                                             if x != v]
 
-        return unreleased_versions, default_fix_versions
+        return all_versions, default_fix_versions
 
     def resolve(self, fix_versions, comment):
-        cur_status = self.issue.fields.status.name
-        cur_summary = self.issue.fields.summary
-        cur_assignee = self.issue.fields.assignee
-        if cur_assignee is None:
-            cur_assignee = "NOT ASSIGNED!!!"
-        else:
-            cur_assignee = cur_assignee.displayName
+        fields = self.issue.fields
+        cur_status = fields.status.name
 
         if cur_status == "Resolved" or cur_status == "Closed":
             self.cmd.fail("JIRA issue %s already has status '%s'"
                           % (self.jira_id, cur_status))
-        print("=== JIRA %s ===" % self.jira_id)
-        print("summary\t\t%s\nassignee\t%s\nstatus\t\t%s\nurl\t\t%s/%s\n"
-              % (cur_summary, cur_assignee, cur_status,
-                 '/'.join((JIRA_API_BASE, 'browse')),
-                 self.jira_id))
+
+        console_output = format_resolved_issue_status(self.jira_id, cur_status,
+                                                      fields.summary,
+                                                      fields.assignee,
+                                                      fields.components)
+        print(console_output)
 
         resolve = [x for x in self.jira_con.transitions(self.jira_id)
                    if x['name'] == "Resolve Issue"][0]
@@ -195,6 +191,28 @@ class JiraIssue(object):
         print("Successfully resolved %s!" % (self.jira_id))
 
 
+def format_resolved_issue_status(jira_id, status, summary, assignee,
+                                 components):
+    if assignee is None:
+        assignee = "NOT ASSIGNED!!!"
+    else:
+        assignee = assignee.displayName
+
+    if len(components) == 0:
+        components = 'NO COMPONENTS!!!'
+    else:
+        components = ', '.join((x.name for x in components))
+
+    return """=== JIRA {} ===
+Summary\t\t{}
+Assignee\t{}
+Components\t{}
+Status\t\t{}
+URL\t\t{}/{}""".format(jira_id, summary, assignee, components, status,
+                       '/'.join((JIRA_API_BASE, 'browse')),
+                       jira_id)
+
+
 class GitHubAPI(object):
 
     def __init__(self, project_name):
@@ -376,15 +394,26 @@ class PullRequest(object):
         return merge_hash
 
 
-def cli():
-    # Location of your Arrow git clone
-    ARROW_HOME = os.path.abspath(os.path.dirname(__file__))
-    PROJECT_NAME = os.environ.get('ARROW_PROJECT_NAME') or 'arrow'
-    print("ARROW_HOME = " + ARROW_HOME)
-    print("PROJECT_NAME = " + PROJECT_NAME)
+def prompt_for_fix_version(cmd, jira_issue):
+    (all_versions,
+     default_fix_versions) = jira_issue.get_candidate_fix_versions()
 
-    cmd = CommandInput()
+    default_fix_versions = ",".join(default_fix_versions)
+
+    issue_fix_versions = cmd.prompt("Enter comma-separated "
+                                    "fix version(s) [%s]: "
+                                    % default_fix_versions)
+    if issue_fix_versions == "":
+        issue_fix_versions = default_fix_versions
+    issue_fix_versions = issue_fix_versions.replace(" ", "").split(",")
+
+    def get_version_json(version_str):
+        return [x for x in all_versions if x.name == version_str][0].raw
 
+    return [get_version_json(v) for v in issue_fix_versions]
+
+
+def connect_jira(cmd):
     # ASF JIRA username
     jira_username = os.environ.get("JIRA_USERNAME")
 
@@ -400,6 +429,19 @@ def cli():
                                     "please enter "
                                     "your JIRA password:")
 
+    return jira.client.JIRA({'server': JIRA_API_BASE},
+                            basic_auth=(jira_username, jira_password))
+
+
+def cli():
+    # Location of your Arrow git clone
+    ARROW_HOME = os.path.abspath(os.path.dirname(__file__))
+    PROJECT_NAME = os.environ.get('ARROW_PROJECT_NAME') or 'arrow'
+    print("ARROW_HOME = " + ARROW_HOME)
+    print("PROJECT_NAME = " + PROJECT_NAME)
+
+    cmd = CommandInput()
+
     pr_num = input("Which pull request would you like to merge? (e.g. 34): ")
 
     # Remote name which points to the GitHub site
@@ -407,10 +449,9 @@ def cli():
 
     os.chdir(ARROW_HOME)
 
-    jira_con = jira.client.JIRA({'server': JIRA_API_BASE},
-                                basic_auth=(jira_username, jira_password))
     github_api = GitHubAPI(PROJECT_NAME)
 
+    jira_con = connect_jira(cmd)
     pr = PullRequest(cmd, github_api, git_remote, jira_con, pr_num)
 
     if pr.is_merged:
@@ -436,21 +477,7 @@ def cli():
            "https://github.com/apache/" + PROJECT_NAME + "/pull",
            pr_num))
 
-    versions, default_fix_versions = pr.jira_issue.get_candidate_fix_versions()
-
-    default_fix_versions = ",".join(default_fix_versions)
-
-    issue_fix_versions = cmd.prompt("Enter comma-separated "
-                                    "fix version(s) [%s]: "
-                                    % default_fix_versions)
-    if issue_fix_versions == "":
-        issue_fix_versions = default_fix_versions
-    issue_fix_versions = issue_fix_versions.replace(" ", "").split(",")
-
-    def get_version_json(version_str):
-        return [x for x in versions if x.name == version_str][0].raw
-
-    fix_versions_json = [get_version_json(v) for v in issue_fix_versions]
+    fix_versions_json = prompt_for_fix_version(cmd, pr.jira_issue)
     pr.jira_issue.resolve(fix_versions_json, jira_comment)
 
 
diff --git a/dev/test_merge_arrow_pr.py b/dev/test_merge_arrow_pr.py
index f69cafe..ce5470b 100644
--- a/dev/test_merge_arrow_pr.py
+++ b/dev/test_merge_arrow_pr.py
@@ -25,22 +25,31 @@ import merge_arrow_pr
 
 
 FakeIssue = namedtuple('issue', ['fields'])
-FakeFields = namedtuple('fields', ['status', 'summary', 'assignee'])
+FakeFields = namedtuple('fields', ['status', 'summary', 'assignee',
+                                   'components'])
 FakeAssignee = namedtuple('assignee', ['displayName'])
 FakeStatus = namedtuple('status', ['name'])
+FakeComponent = namedtuple('component', ['name'])
 FakeProjectVersion = namedtuple('version', ['name', 'raw'])
 
-SOURCE_VERSIONS = [FakeProjectVersion('JS-0.4.0', {'released': False}),
-                   FakeProjectVersion('0.11.0', {'released': False}),
-                   FakeProjectVersion('0.12.0', {'released': False}),
-                   FakeProjectVersion('0.10.0', {'released': True}),
-                   FakeProjectVersion('0.9.0', {'released': True})]
+RAW_VERSION_JSON = [
+    {'version': 'JS-0.4.0', 'released': False},
+    {'version': '0.11.0', 'released': False},
+    {'version': '0.12.0', 'released': False},
+    {'version': '0.10.0', 'released': True},
+    {'version': '0.9.0', 'released': True}
+]
+
+
+SOURCE_VERSIONS = [FakeProjectVersion(raw['version'], raw)
+                   for raw in RAW_VERSION_JSON]
 
 TRANSITIONS = [{'name': 'Resolve Issue', 'id': 1}]
 
 jira_id = 'ARROW-1234'
 status = FakeStatus('In Progress')
-fields = FakeFields(status, 'issue summary', FakeAssignee('groundhog'))
+fields = FakeFields(status, 'issue summary', FakeAssignee('groundhog'),
+                    [FakeComponent('C++'), FakeComponent('Format')])
 FAKE_ISSUE_1 = FakeIssue(fields)
 
 
@@ -66,6 +75,9 @@ class FakeJIRA:
             'fixVersions': fixVersions
         }
 
+    def get_candidate_fix_versions(self):
+        return SOURCE_VERSIONS, ['0.12.0']
+
     def project_versions(self, project):
         return self._project_versions
 
@@ -91,11 +103,7 @@ def test_jira_fix_versions():
 
     issue = merge_arrow_pr.JiraIssue(jira, 'ARROW-1234', 'ARROW', FakeCLI())
     all_versions, default_versions = issue.get_candidate_fix_versions()
-
-    expected = sorted([x for x in SOURCE_VERSIONS
-                       if not x.raw['released']],
-                      key=lambda x: x.name, reverse=True)
-    assert all_versions == expected
+    assert all_versions == SOURCE_VERSIONS
     assert default_versions == ['0.11.0']
 
 
@@ -147,9 +155,21 @@ def test_jira_resolve_non_mainline():
     }
 
 
+def test_jira_resolve_released_fix_version():
+    # ARROW-5083
+    jira = FakeJIRA(issue=FAKE_ISSUE_1,
+                    project_versions=SOURCE_VERSIONS,
+                    transitions=TRANSITIONS)
+
+    cmd = FakeCLI(responses=['0.9.0'])
+    fix_versions_json = merge_arrow_pr.prompt_for_fix_version(cmd, jira)
+    assert fix_versions_json == [RAW_VERSION_JSON[-1]]
+
+
 def test_jira_already_resolved():
     status = FakeStatus('Resolved')
-    fields = FakeFields(status, 'issue summary', FakeAssignee('groundhog'))
+    fields = FakeFields(status, 'issue summary', FakeAssignee('groundhog'),
+                        [FakeComponent('Java')])
     issue = FakeIssue(fields)
 
     jira = FakeJIRA(issue=issue,
@@ -162,3 +182,30 @@ def test_jira_already_resolved():
     with pytest.raises(Exception,
                        match="ARROW-1234 already has status 'Resolved'"):
         issue.resolve(fix_versions, "")
+
+
+def test_jira_output_no_components():
+    # ARROW-5472
+    status = 'Interesting work'
+    components = []
+    output = merge_arrow_pr.format_resolved_issue_status(
+        'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'),
+        components)
+
+    assert output == """=== JIRA ARROW-1234 ===
+Summary\t\tInteresting work
+Assignee\tFoo Bar
+Components\tNO COMPONENTS!!!
+Status\t\tResolved
+URL\t\thttps://issues.apache.org/jira/browse/ARROW-1234"""
+
+    output = merge_arrow_pr.format_resolved_issue_status(
+        'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'),
+        [FakeComponent('C++'), FakeComponent('Python')])
+
+    assert output == """=== JIRA ARROW-1234 ===
+Summary\t\tInteresting work
+Assignee\tFoo Bar
+Components\tC++, Python
+Status\t\tResolved
+URL\t\thttps://issues.apache.org/jira/browse/ARROW-1234"""