From 6623f8316a33016078bfae755b2226cdeaca93cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A9ri=20Le=20Bouder?= Date: Tue, 16 Jun 2020 16:25:29 -0400 Subject: [PATCH] github: use change.message in squahsed commit message So far, github use the subject of the first change in the commit message body. This is most of the time redundant with the PR title. Ths idea is to use instead the change body. Change-Id: I9a2bc8aa3ff60a28b2ed62a4ee5fa9f7a2c9dcda --- tests/base.py | 7 ++++--- tests/fakegithub.py | 1 + tests/unit/test_github_driver.py | 10 ++++++++-- zuul/driver/github/githubconnection.py | 1 + zuul/driver/github/githubmodel.py | 1 + zuul/driver/github/githubreporter.py | 14 ++++++++------ 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/tests/base.py b/tests/base.py index 387775e3ca..4727715651 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1798,7 +1798,7 @@ class FakeGithubPullRequest(object): def __init__(self, github, number, project, branch, subject, upstream_root, files=[], number_of_commits=1, - writers=[], body=None, draft=False): + writers=[], body=None, body_text=None, draft=False): """Creates a new PR with several commits. Sends an event about opened PR.""" self.github = github @@ -1808,6 +1808,7 @@ class FakeGithubPullRequest(object): self.branch = branch self.subject = subject self.body = body + self.body_text = body_text self.draft = draft self.number_of_commits = 0 self.upstream_root = upstream_root @@ -2200,11 +2201,11 @@ class FakeGithubConnection(githubconnection.GithubConnection): self.zuul_web_port = port def openFakePullRequest(self, project, branch, subject, files=[], - body=None, draft=False): + body=None, body_text=None, draft=False): self.pr_number += 1 pull_request = FakeGithubPullRequest( self, self.pr_number, project, branch, subject, self.upstream_root, - files=files, body=body, draft=draft) + files=files, body=body, body_text=body_text, draft=draft) self.pull_requests[self.pr_number] = pull_request return pull_request diff --git a/tests/fakegithub.py b/tests/fakegithub.py index 558d23471a..3909ab0111 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -521,6 +521,7 @@ class FakePull(object): }, 'merged': pr.is_merged, 'body': pr.body, + 'body_text': pr.body_text, 'changed_files': len(pr.files), 'labels': [{'name': l} for l in pr.labels] } diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 0961ede2b7..98066e5140 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -630,12 +630,18 @@ class TestGithubDriver(ZuulTestCase): def test_report_pull_merge(self): # pipeline merges the pull request on success A = self.fake_github.openFakePullRequest('org/project', 'master', - 'PR title') + 'PR title', + body='I shouldnt be seen', + body_text='PR body') self.fake_github.emitEvent(A.getCommentAddedEvent('merge me')) self.waitUntilSettled() self.assertTrue(A.is_merged) self.assertThat(A.merge_message, - MatchesRegex(r'.*PR title.*', re.DOTALL)) + MatchesRegex(r'.*PR title\n\nPR body.*', re.DOTALL)) + self.assertThat(A.merge_message, + Not(MatchesRegex( + r'.*I shouldnt be seen.*', + re.DOTALL))) self.assertEqual(len(A.comments), 0) # pipeline merges the pull request on success after failure diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index ebcf04c327..63d51d607e 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -1374,6 +1374,7 @@ class GithubConnection(BaseConnection): else: message = change.title change.message = message + change.body_text = change.pr.get("body_text") # Note(tobiash): The updated_at timestamp is a moving target that is # not bound to the pull request 'version' we can solve that by just not diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index 9183a5df13..f5945a6c63 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -34,6 +34,7 @@ class PullRequest(Change): self.pr = None self.updated_at = None self.title = None + self.body_text = None self.reviews = [] self.files = [] self.labels = [] diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index 8062db04ec..40dbaf5bc3 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -265,10 +265,12 @@ class GithubReporter(BaseReporter): zuul_event_id=item.event) def _formatMergeMessage(self, change): - message = '' - + message = [] if change.title: - message += change.title + message.append(change.title) + if change.body_text: + message.append(change.body_text) + merge_message = "\n\n".join(message) if change.reviews: review_users = [] @@ -276,10 +278,10 @@ class GithubReporter(BaseReporter): name = r['by']['username'] email = r['by']['email'] review_users.append('Reviewed-by: {} <{}>'.format(name, email)) - message += '\n\n' - message += '\n'.join(review_users) + merge_message += '\n\n' + merge_message += '\n'.join(review_users) - return message + return merge_message def getSubmitAllowNeeds(self): """Get a list of code review labels that are allowed to be