MRG: Merge pull request #50 from octue/fix/get-commits-from-pull-request

Get commits from pull request and remove LAST_PULL_REQUEST stop point option

.github/workflows/update-pull-request.yml

@@ -17,9 +17,6 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
-      with:
-        # Set fetch-depth to 0 to fetch all commit history (necessary for compiling pull request description).
-        fetch-depth: 0
     - name: Install release note compiler
       run: pip install git+https://github.com/octue/conventional-commits
     - name: Compile new pull request description

README.md

@@ -193,7 +193,6 @@ A command-line tool that compiles release notes and pull request descriptions fr
 stopping at the specified stop point. The stop point can be one of:
 * The first commit of the pull request - `PULL_REQUEST_START`
 * The last semantically-versioned release tagged in the git history relative to the checked-out branch - `LAST_RELEASE`
-* The last merged pull request in the git history relative to the checked-out branch - `LAST_PULL_REQUEST`
 
 If breaking changes are indicated in any of the commit messages' bodies via `BREAKING CHANGE` or `BREAKING-CHANGE`,
 these commits are marked and a warning is added to the top of the release notes.
@@ -209,10 +208,10 @@ Note that these comment lines are invisible in rendered markdown.
 ```shell
 usage: compile-release-notes [-h] [--pull-request-url PULL_REQUEST_URL] [--api-token API_TOKEN] [--header HEADER] [--list-item-symbol LIST_ITEM_SYMBOL]
                              [--no-link-to-pull-request]
-                             {LAST_RELEASE,LAST_PULL_REQUEST,PULL_REQUEST_START}
+                             {LAST_RELEASE,PULL_REQUEST_START}
 
 positional arguments:
-  {LAST_RELEASE,LAST_PULL_REQUEST,PULL_REQUEST_START}
+  {LAST_RELEASE,PULL_REQUEST_START}
                         The point in the git history to stop compiling commits into the release notes.
 
 optional arguments:

conventional_commits/check_commit_message.py

@@ -65,7 +65,7 @@ def __init__(
         self,
         allowed_commit_codes=None,
         maximum_header_length=72,
-        valid_header_ending_pattern=r"[A-Za-z\d]",
+        valid_header_ending_pattern=r"[A-Za-z\d\"\']",
         require_body=False,
         maximum_body_line_length=72,
     ):

conventional_commits/compile_release_notes.py

@@ -14,15 +14,13 @@
 
 
 LAST_RELEASE = "LAST_RELEASE"
-LAST_PULL_REQUEST = "LAST_PULL_REQUEST"
 PULL_REQUEST_START = "PULL_REQUEST_START"
-STOP_POINTS = (LAST_RELEASE, LAST_PULL_REQUEST, PULL_REQUEST_START)
+STOP_POINTS = (LAST_RELEASE, PULL_REQUEST_START)
 
 BREAKING_CHANGE_INDICATOR = "**BREAKING CHANGE:** "
 
 COMMIT_REF_MERGE_PATTERN = re.compile(r"Merge [0-9a-f]+ into [0-9a-f]+")
 SEMANTIC_VERSION_PATTERN = re.compile(r"tag: (\d+\.\d+\.\d+)")
-PULL_REQUEST_INDICATOR = "Merge pull request #"
 
 COMMIT_CODES_TO_HEADINGS_MAPPING = {
     "FEA": "### New features",
@@ -54,7 +52,7 @@ class ReleaseNotesCompiler:
     comment lines `<!--- START AUTOGENERATED NOTES --->` and `<!--- END AUTOGENERATED NOTES --->` will be replaced -
     anything outside of this will appear in the new release notes.
 
-    :param str stop_point: the point in the git history up to which commit messages should be used - should be either "LAST_RELEASE" or "LAST_PULL_REQUEST"
+    :param str stop_point: the point in the git history up to which commit messages should be used - should be either "LAST_RELEASE" or "PULL_REQUEST_START"
     :param str|None pull_request_url: GitHub API URL for the pull request - this can be accessed in a GitHub workflow as ${{ github.event.pull_request.url }}
     :param str|None api_token: GitHub API token - this can be accessed in a GitHub workflow as ${{ secrets.GITHUB_TOKEN }}
     :param str header: the header to put above the autogenerated release notes, including any markdown styling (defaults to "## Contents")
@@ -93,12 +91,6 @@ def __init__(
         self.commit_codes_to_headings_mapping = commit_codes_to_headings_mapping or COMMIT_CODES_TO_HEADINGS_MAPPING
         self.include_link_to_pull_request = include_link_to_pull_request
 
-        if self.stop_point == PULL_REQUEST_START:
-            if self.current_pull_request is not None:
-                self.base_branch = self.current_pull_request["base"]["ref"]
-            else:
-                self.stop_point = LAST_RELEASE
-
         logger.info(f"Using {self.stop_point!r} stop point.")
 
     def compile_release_notes(self):
@@ -117,8 +109,11 @@ def compile_release_notes(self):
         if self.previous_notes and SKIP_INDICATOR in self.previous_notes:
             return self.previous_notes
 
-        git_log = self._get_git_log()
-        parsed_commits, unparsed_commits = self._parse_commit_messages(git_log)
+        if self.current_pull_request:
+            parsed_commits, unparsed_commits = self._parse_commit_messages_from_github()
+        else:
+            parsed_commits, unparsed_commits = self._parse_commit_messages_from_git_log()
+
         categorised_commit_messages = self._categorise_commit_messages(parsed_commits, unparsed_commits)
         autogenerated_release_notes = self._build_release_notes(categorised_commit_messages)
 
@@ -143,7 +138,7 @@ def _get_current_pull_request(self, pull_request_url, api_token):
 
         :param str pull_request_url: the GitHub API URL for the pull request
         :param str|None api_token: GitHub API token
-        :return dict:
+        :return dict|None:
         """
         if api_token is None:
             headers = {}
@@ -153,13 +148,18 @@ def _get_current_pull_request(self, pull_request_url, api_token):
         response = requests.get(pull_request_url, headers=headers)
 
         if response.status_code == 200:
-            return response.json()
+            pull_request = response.json()
+            pull_request["commits"] = requests.get(pull_request["commits_url"], headers=headers).json()
+            return pull_request
 
         logger.warning(
             f"Pull request could not be accessed; resorting to using {LAST_RELEASE} stop point.\n"
             f"{response.status_code}: {response.text}."
         )
 
+        self.stop_point = LAST_RELEASE
+        return None
+
     def _get_git_log(self):
         """Get the one-line decorated git log formatted in the pattern of "hash|§header|§body|§decoration@@@".
 
@@ -171,30 +171,27 @@ def _get_git_log(self):
         * The specific characters used for the delimiters have been chosen so that they are very uncommon to reduce
           delimiting errors
 
-        :return str:
+        :return list(str):
         """
         return (
             subprocess.run(["git", "log", "--pretty=format:%h|§%s|§%b|§%d@@@"], capture_output=True)
             .stdout.strip()
             .decode()
-        )
+        ).split("@@@")
 
-    def _parse_commit_messages(self, formatted_one_line_git_log):
-        """Parse commit messages from the git log (formatted using `--pretty=format:%s|%d`) until the stop point is
-        reached. The parsed commit messages are returned separately to any that fail to parse.
+    def _parse_commit_messages_from_git_log(self):
+        """Parse commit messages from the git log (formatted using `--pretty=format:%h|§%s|§%b|§%d@@@`) until the stop
+        point is reached. The parsed commit messages are returned separately to any that fail to parse.
 
-        :param str formatted_one_line_git_log:
         :return list(tuple), list(str):
         """
         parsed_commits = []
         unparsed_commits = []
 
-        commits = formatted_one_line_git_log.split("@@@")
-
-        for commit in commits:
+        for commit in self._get_git_log():
             hash, header, body, decoration = commit.split("|§")
 
-            if self._is_stop_point(header, decoration):
+            if "tag" in decoration and bool(SEMANTIC_VERSION_PATTERN.search(decoration)):
                 break
 
             # A colon separating the commit code from the commit header is required - keep commit messages that
@@ -210,27 +207,39 @@ def _parse_commit_messages(self, formatted_one_line_git_log):
             code, *header = header.split(":")
             header = ":".join(header)
 
-            parsed_commits.append((code.strip(), header.strip(), body.strip(), decoration.strip()))
+            parsed_commits.append((code.strip(), header.strip(), body.strip()))
 
         return parsed_commits, unparsed_commits
 
-    def _is_stop_point(self, message, decoration):
-        """Check if this commit header is the stop point for collecting commits for the release notes.
+    def _parse_commit_messages_from_github(self):
+        """Parse commit messages from the GitHub pull request. The parsed commit messages are returned separately to
+        any that fail to parse.
 
-        :param str message:
-        :param str decoration:
-        :return bool:
+        :return list(tuple), list(str):
         """
-        if self.stop_point == PULL_REQUEST_START:
-            if self.base_branch in decoration:
-                return True
+        parsed_commits = []
+        unparsed_commits = []
+
+        for commit in self.current_pull_request["commits"]:
+            header, *body = commit["commit"]["message"].split("\n")
+            body = "\n".join(body)
+
+            # A colon separating the commit code from the commit header is required - keep commit messages that
+            # don't conform to this but put them into an unparsed category. Ignore commits that are merges of one
+            # commit ref into another (GitHub Actions produces these - they don't appear in the actual history of
+            # the branch so can be safely ignored when making release notes).
+            if ":" not in header:
+                if not COMMIT_REF_MERGE_PATTERN.search(header):
+                    unparsed_commits.append(header.strip())
+                continue
+
+            # Allow commit headers with extra colons.
+            code, *header = header.split(":")
+            header = ":".join(header)
 
-        elif self.stop_point == LAST_RELEASE:
-            if "tag" in decoration:
-                return bool(SEMANTIC_VERSION_PATTERN.search(decoration))
+            parsed_commits.append((code.strip(), header.strip(), body.strip()))
 
-        elif self.stop_point == LAST_PULL_REQUEST:
-            return PULL_REQUEST_INDICATOR in message
+        return parsed_commits, unparsed_commits
 
     def _categorise_commit_messages(self, parsed_commits, unparsed_commits):
         """Categorise the commit messages into headed sections. Unparsed commits are put under an "uncategorised"
@@ -243,7 +252,7 @@ def _categorise_commit_messages(self, parsed_commits, unparsed_commits):
         release_notes = {heading: [] for heading in self.commit_codes_to_headings_mapping.values()}
         release_notes[BREAKING_CHANGE_COUNT_KEY] = 0
 
-        for code, header, body, _ in parsed_commits:
+        for code, header, body in parsed_commits:
             try:
                 if any(indicator in body for indicator in CONVENTIONAL_COMMIT_BREAKING_CHANGE_INDICATORS):
                     commit_note = BREAKING_CHANGE_INDICATOR + header

examples/release_notes_compiler/workflow.yml

@@ -11,9 +11,6 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
-      with:
-        # Set fetch-depth to 0 to fetch all commit history (necessary for compiling pull request description).
-        fetch-depth: 0
     - name: Install release note compiler
       run: pip install git+https://github.com/octue/conventional-commits
     - name: Compile new pull request description

setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = conventional_commits
-version = 0.4.1
+version = 0.5.0
 description = A pre-commit hook, semantic version checker, and release note compiler for facilitating continuous deployment via Conventional Commits.
 long_description = file: README.md
 long_description_content_type = text/markdown

tests/test_check_commit_message.py

@@ -44,6 +44,10 @@ def test_non_blank_header_separator_line_raises_error(self):
         with self.assertRaises(ValueError):
             ConventionalCommitMessageChecker().check_commit_message(["FIX: Fix this bug", "Body started too early."])
 
+    def test_valid_header_ending(self):
+        """Test that a commit message with a valid header ending is ok."""
+        ConventionalCommitMessageChecker().check_commit_message(['REV: Reverts "FIX: Fix a bug"'])
+
     def test_with_body_when_body_not_required(self):
         """Test that a commit message with a valid header and body when a body is not required is ok."""
         ConventionalCommitMessageChecker().check_commit_message(["FIX: Fix this bug", "", "This is the body."])