From e3d6cb0724a413a2279cc9e08b31f8adf2acec44 Mon Sep 17 00:00:00 2001 From: Simon Westphahl Date: Wed, 1 Mar 2023 14:03:14 +0100 Subject: Don't add PR title in commit message on squash Github will use the PR title as the commit subject for squash merges, so we don't need include the title once again in the commit description. Change-Id: Id5a00701c236235f5a49abd025bcfad1b2da916c --- tests/fakegithub.py | 2 +- tests/unit/test_github_driver.py | 7 ++++++- zuul/driver/github/githubreporter.py | 13 ++++++++----- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/fakegithub.py b/tests/fakegithub.py index 725c083e2..25dcb15da 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -730,7 +730,7 @@ class FakeGithubSession(object): 'message': 'Merge not allowed because of fake reason', } return FakeResponse(data, 405, 'Method not allowed') - pr.setMerged(json["commit_message"]) + pr.setMerged(json.get("commit_message", "")) return FakeResponse({"merged": True}, 200) return FakeResponse(None, 404) diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index fb46aa7d1..0b3b6369b 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1430,7 +1430,9 @@ class TestGithubDriver(ZuulTestCase): repo._set_branch_protection( 'master', contexts=['tenant-one/check', 'tenant-one/gate']) - A = self.fake_github.openFakePullRequest('org/project', 'master', 'A') + pr_description = "PR description" + A = self.fake_github.openFakePullRequest('org/project', 'master', 'A', + body_text=pr_description) self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) self.waitUntilSettled() @@ -1448,6 +1450,9 @@ class TestGithubDriver(ZuulTestCase): merges = [report for report in self.fake_github.github_data.reports if report[2] == 'merge'] assert (len(merges) == 1 and merges[0][3] == 'squash') + # Assert that we won't duplicate the PR title in the merge + # message description. + self.assertEqual(A.merge_message, pr_description) @simple_layout('layouts/basic-github.yaml', driver='github') def test_invalid_event(self): diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index 1f44303bd..396516038 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -193,13 +193,13 @@ class GithubReporter(BaseReporter): self.log.warning('Merge mode %s not supported by Github', mode) raise MergeFailure('Merge mode %s not supported by Github' % mode) - merge_mode = self.merge_modes[merge_mode] project = item.change.project.name pr_number = item.change.number sha = item.change.patchset log.debug('Reporting change %s, params %s, merging via API', item.change, self.config) - message = self._formatMergeMessage(item.change) + message = self._formatMergeMessage(item.change, merge_mode) + merge_mode = self.merge_modes[merge_mode] for i in [1, 2]: try: @@ -319,10 +319,13 @@ class GithubReporter(BaseReporter): self.connection.unlabelPull(project, pr_number, label, zuul_event_id=item.event) - def _formatMergeMessage(self, change): + def _formatMergeMessage(self, change, merge_mode): message = [] - if change.title: - message.append(change.title) + # For squash merges we don't need to add the title to the body + # as it will already be set as the commit subject. + if merge_mode != model.MERGER_SQUASH_MERGE: + if change.title: + message.append(change.title) if change.body_text: message.append(change.body_text) merge_message = "\n\n".join(message) -- cgit v1.2.1