Do not suggest setting Fix Version for patch releases by default

wesm · wesm · commit 27f566412eec · 2019-08-19T16:59:00.000-05:00
diff --git a/ci/travis_lint.sh b/ci/travis_lint.sh
@@ -23,15 +23,14 @@ set -ex
 export ARROW_TRAVIS_USE_TOOLCHAIN=0
 source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
 
-pip install pre_commit
+pip install pre_commit cmake_format==0.5.2 pytest
 pre-commit install
 
 # TODO: Move more checks into pre-commit as this gives a nice summary
 # and doesn't abort on the first failed check.
 pre-commit run hadolint -a
 
 # CMake formatting check
-pip install cmake_format==0.5.2
 $TRAVIS_BUILD_DIR/run-cmake-format.py --check
 
 # C++ code linting
@@ -54,7 +53,10 @@ FLAKE8="python3 -m flake8"
 python3 -m pip install -q flake8
 
 if [ "$ARROW_CI_DEV_AFFECTED" != "0" ]; then
-  $FLAKE8 --count $ARROW_DEV_DIR
+  pushd $ARROW_DEV_DIR
+  $FLAKE8 --count .
+  py.test test_merge_arrow_pr.py
+  popd
 fi
 
 if [ "$ARROW_CI_INTEGRATION_AFFECTED" != "0" ]; then
diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py
@@ -160,23 +160,20 @@ def get_candidate_fix_versions(self, merge_branches=('master',)):
         mainline_versions = [x for x in unreleased_versions
                              if mainline_version_regex.match(x.name)]
 
+        mainline_non_patch_versions = []
+        for v in mainline_versions:
+            (major, minor, patch) = v.name.split(".")
+            if patch == "0":
+                mainline_non_patch_versions.append(v)
+
+        if len(mainline_versions) > len(mainline_non_patch_versions):
+            # If there is a non-patch release, suggest that instead
+            mainline_versions = mainline_non_patch_versions
+
         default_fix_versions = [
             fix_version_from_branch(x, mainline_versions).name
             for x in merge_branches]
 
-        for v in default_fix_versions:
-            # Handles the case where we have forked a release branch but not
-            # yet made the release.  In this case, if the PR is committed to
-            # the master branch and the release branch, we only consider the
-            # release branch to be the fix version. E.g. it is not valid to
-            # have both 1.1.0 and 1.0.0 as fix versions.
-            (major, minor, patch) = v.split(".")
-            if patch == "0":
-                previous = "%s.%s.%s" % (major, int(minor) - 1, 0)
-                if previous in default_fix_versions:
-                    default_fix_versions = [x for x in default_fix_versions
-                                            if x != v]
-
         return all_versions, default_fix_versions
 
     def resolve(self, fix_versions, comment):
@@ -200,26 +197,29 @@ def resolve(self, fix_versions, comment):
 
     def show(self):
         fields = self.issue.fields
+        print(format_jira_output(self.jira_id, fields.status.name,
+                                 fields.summary, fields.assignee,
+                                 fields.components))
 
-        assignee = fields.assignee
-        if assignee is None:
-            assignee = "NOT ASSIGNED!!!"
-        else:
-            assignee = assignee.displayName
 
-        components = fields.components
-        if len(components) == 0:
-            components = 'NO COMPONENTS!!!'
-        else:
-            components = ', '.join((x.name for x in components))
-
-        print("=== JIRA {} ===".format(self.jira_id))
-        print("Summary\t\t{}".format(fields.summary))
-        print("Assignee\t{}".format(assignee))
-        print("Components\t{}".format(components))
-        print("Status\t\t{}".format(fields.status.name))
-        print("URL\t\t{}/{}".format('/'.join((JIRA_API_BASE, 'browse')),
-                                    self.jira_id))
+def format_jira_output(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):
diff --git a/dev/test_merge_arrow_pr.py b/dev/test_merge_arrow_pr.py
@@ -107,6 +107,22 @@ def test_jira_fix_versions():
     assert default_versions == ['0.11.0']
 
 
+def test_jira_no_suggest_patch_release():
+    versions_json = [
+        {'version': '0.11.1', 'released': False},
+        {'version': '0.12.0', 'released': False},
+    ]
+
+    versions = [FakeProjectVersion(raw['version'], raw)
+                for raw in versions_json]
+
+    jira = FakeJIRA(project_versions=versions, transitions=TRANSITIONS)
+    issue = merge_arrow_pr.JiraIssue(jira, 'ARROW-1234', 'ARROW', FakeCLI())
+    all_versions, default_versions = issue.get_candidate_fix_versions()
+    assert all_versions == versions
+    assert default_versions == ['0.12.0']
+
+
 def test_jira_invalid_issue():
     class Mock:
 
@@ -188,7 +204,7 @@ def test_jira_output_no_components():
     # ARROW-5472
     status = 'Interesting work'
     components = []
-    output = merge_arrow_pr.format_resolved_issue_status(
+    output = merge_arrow_pr.format_jira_output(
         'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'),
         components)
 
@@ -199,7 +215,7 @@ def test_jira_output_no_components():
 Status\t\tResolved
 URL\t\thttps://issues.apache.org/jira/browse/ARROW-1234"""
 
-    output = merge_arrow_pr.format_resolved_issue_status(
+    output = merge_arrow_pr.format_jira_output(
         'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'),
         [FakeComponent('C++'), FakeComponent('Python')])
 
