Merge pull request #44 from opalmer/release_bugfixes

Release Tools Had Wrong URLs and Broken Tests

.ci/travis/script.sh

@@ -8,8 +8,9 @@ if [[ $PYLINT == "1" ]]; then
     # more noisy checks that don't effect quality for testing
     # purposes.
     pylint tests \
-        --disable missing-docstring,invalid-name \
-        --disable protected-access,no-self-use,unused-argument
+        --disable missing-docstring,invalid-name,too-many-arguments \
+        --disable protected-access,no-self-use,unused-argument \
+        --disable too-few-public-methods
 fi
 
 if [[ $READTHEDOCS == "1" ]]; then

pywincffi/dev/release.py

@@ -193,6 +193,9 @@ def download(cls, url, path=None, chunk_size=1024):
 
         return path
 
+Issue = namedtuple(
+    "Issue", ("issue", "closed", "labels", "type", "number", "url", "title"))
+
 
 class GitHubAPI(object):  # pylint: disable=too-many-instance-attributes
     """
@@ -203,7 +206,11 @@ class GitHubAPI(object):  # pylint: disable=too-many-instance-attributes
     PROJECT = "pywincffi"
     REPO_NAME = "opalmer/%s" % PROJECT
 
-    def __init__(self, version, branch="master"):
+    # NOTE: `repo` is for testing purposes.
+    def __init__(self, version, branch=None, repo_=None):
+        if branch is None:
+            branch = "master"
+
         self.version = version
         self.branch = branch
         self.read_the_docs = \
@@ -221,9 +228,12 @@ def __init__(self, version, branch="master"):
                 "pywincffi.github_token is not set in the config")
 
         self.hub = Github(login_or_token=github_token)
-        self.repo = self.hub.get_repo(self.REPO_NAME)
+        self.repo = repo_
+
+        if repo_ is None:  # pragma: no cover
+            self.repo = self.hub.get_repo(self.REPO_NAME)
 
-        for milestone in self.repo.get_milestones():
+        for milestone in self.repo.get_milestones(state="all"):
             if milestone.title == self.version:
                 self.milestone = milestone
                 break
@@ -236,6 +246,46 @@ def commit(self):
         branch = self.repo.get_branch(self.branch)
         return branch.commit.sha
 
+    def issues(self):
+        """
+        Generator which yields all issues associated with the milestone as well
+        as some extra information
+        """
+        issues = self.repo.get_issues(milestone=self.milestone, state="all")
+
+        for issue in issues:
+            # Determine the type of issue this is.  This is mostly used
+            # when building the release message.
+            labels = set(label.name for label in issue.labels)
+            if "in progress" in labels or "in review" in labels:
+                logger.warning(
+                    "Issue %s is still a work in progress", issue.number)
+
+            if "bug" in labels:
+                issue_type = "bugs"
+
+            elif "enhancement" in labels:
+                issue_type = "enhancements"
+
+            elif "documentation" in labels:
+                issue_type = "documentation"
+
+            elif "unittest" in labels:
+                issue_type = "unittests"
+
+            else:
+                issue_type = "other"
+
+            yield Issue(
+                issue=issue,
+                closed=issue.state == "closed",
+                labels=labels,
+                type=issue_type,
+                number=issue.number,
+                url=issue.html_url,
+                title=issue.title
+            )
+
     def release_message(self):
         """Produces release message for :meth:`create_release` to use."""
         output = StringIO()
@@ -252,46 +302,36 @@ def release_message(self):
         print("Pull requests and issues associated with this release.",
               file=output)
         print("", file=output)
-        issues = {
-            "bugs": [],
-            "enhancements": [],
-            "unittests": [],
-            "documentation": [],
-            "other": []
-        }
-        for issue in self.repo.get_issues(
-                milestone=self.milestone, state="all"):
-            for label in issue.labels:
-                if label.name == "bug":
-                    issues["bugs"].append(issue)
-                    break
-                if label.name == "enhancement":
-                    issues["enhancements"].append(issue)
-                    break
-                if label.name == "documentation":
-                    issues["documentation"].append(issue)
-                    break
-                if label.name == "unittest":
-                    issues["unittests"].append(issue)
-                    break
-            else:
-                issues["other"].append(issue)
 
-        for value in issues.values():
-            value.reverse()
+        issues = {}
+        for issue in self.issues():
+            issues_by_type = issues.setdefault(issue.type, [])
+            issues_by_type.append(issue)
+
+        for issues_list in issues.values():
+            issues_list.sort(key=lambda item: item.number)
 
         for name in (
                 "enhancements", "bugs", "documentation", "unittests", "other"):
-            if issues[name]:
+            issues_for_name = issues.pop(name, [])
+
+            if issues_for_name:
+                print("", file=output)
                 print("#### %s" % name.title(), file=output)
-                for issue in issues[name.lower()]:
-                    if issue.state != "closed":
-                        logger.warning("Issue %s is not closed!", issue.number)
+                for issue in issues_for_name:
                     print(
-                        "[%s](%s) - %s" % (
-                            issue.number, issue.url, issue.title),
+                        "[{number}]({url}) - {title}".format(
+                            number=issue.number, url=issue.url,
+                            title=issue.title
+                        ),
                         file=output)
 
+        # The for loop above establishes the order we want to see
+        # the various types of issues in.  If a new type is added,
+        # we'll need to update this.
+        if issues:
+            raise ValueError("Unhandled keys: %s" % issues.keys())
+
         return output.getvalue()
 
     def create_release(
@@ -309,14 +349,17 @@ def create_release(
 
         for release in self.repo.get_releases():
             if release.tag_name == self.version:
-                if recreate:
+                if recreate and not dry_run:
                     logger.warning(
                         "Deleting existing release for %s", release.tag_name)
                     release.delete_release()
                     # TODO: make sure we delete the tag too
                 else:
-                    raise RuntimeError(
-                        "A release for %r already exists" % self.version)
+                    message = "A release for %r already exists" % self.version
+                    if dry_run:
+                        logger.error(message)
+                    else:
+                        raise RuntimeError(message)
 
         logger.info("Creating **draft** release %r", self.version)
         message = self.release_message()