Better merge message for GitHub pull reqeusts
* Preserve pull reqeust title in the commit message.
* Add information about the user who triggered the merge.
Change-Id: Ibe0bb5ee273a6ae22fde2e0d71fd10b600dee17f
diff --git a/tests/base.py b/tests/base.py
index 77c0644..c567b03 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -564,6 +564,7 @@
self.updated_at = None
self.head_sha = None
self.is_merged = False
+ self.merge_message = None
self._createPRRef()
self._addCommitToRepo()
self._updateTimeStamp()
@@ -608,6 +609,9 @@
},
'repository': {
'full_name': self.project
+ },
+ 'sender': {
+ 'login': 'ghuser'
}
}
return (name, data)
@@ -643,6 +647,9 @@
},
'label': {
'name': label
+ },
+ 'sender': {
+ 'login': 'ghuser'
}
}
return (name, data)
@@ -653,6 +660,7 @@
'action': 'unlabeled',
'pull_request': {
'number': self.number,
+ 'title': self.subject,
'updated_at': self.updated_at,
'base': {
'ref': self.branch,
@@ -666,6 +674,9 @@
},
'label': {
'name': label
+ },
+ 'sender': {
+ 'login': 'ghuser'
}
}
return (name, data)
@@ -732,6 +743,7 @@
'number': self.number,
'pull_request': {
'number': self.number,
+ 'title': self.subject,
'updated_at': self.updated_at,
'base': {
'ref': self.branch,
@@ -742,6 +754,9 @@
'head': {
'sha': self.head_sha
}
+ },
+ 'sender': {
+ 'login': 'ghuser'
}
}
return (name, data)
@@ -800,6 +815,7 @@
pr = self.pull_requests[number - 1]
data = {
'number': number,
+ 'title': pr.subject,
'updated_at': pr.updated_at,
'base': {
'repo': {
@@ -814,6 +830,14 @@
}
return data
+ def getUser(self, login):
+ data = {
+ 'username': login,
+ 'name': 'Github User',
+ 'email': 'github.user@example.com'
+ }
+ return data
+
def getGitUrl(self, project):
return os.path.join(self.upstream_root, str(project))
@@ -830,7 +854,7 @@
pull_request = self.pull_requests[pr_number - 1]
pull_request.addComment(message)
- def mergePull(self, project, pr_number, sha=None):
+ def mergePull(self, project, pr_number, commit_message='', sha=None):
pull_request = self.pull_requests[pr_number - 1]
if self.merge_failure:
raise Exception('Pull request was not merged')
@@ -839,6 +863,7 @@
raise MergeFailure('Merge was not successful due to mergeability'
' conflict')
pull_request.is_merged = True
+ pull_request.merge_message = commit_message
def setCommitStatus(self, project, sha, state,
url='', description='', context=''):
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py
index 3f567d2..7267b83 100644
--- a/tests/unit/test_github_driver.py
+++ b/tests/unit/test_github_driver.py
@@ -258,10 +258,13 @@
@simple_layout('layouts/merging-github.yaml', driver='github')
def test_report_pull_merge(self):
# pipeline merges the pull request on success
- A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
+ A = self.fake_github.openFakePullRequest('org/project', 'master',
+ 'PR title')
self.fake_github.emitEvent(A.getCommentAddedEvent('merge me'))
self.waitUntilSettled()
self.assertTrue(A.is_merged)
+ self.assertThat(A.merge_message,
+ MatchesRegex('.*PR title.*Reviewed-by.*', re.DOTALL))
# pipeline merges the pull request on success after failure
self.fake_github.merge_failure = True
diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py
index f2a8fc0..e8162cd 100644
--- a/zuul/driver/github/githubconnection.py
+++ b/zuul/driver/github/githubconnection.py
@@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
+import collections
import logging
import hmac
import hashlib
@@ -100,6 +101,7 @@
pr_body = body.get('pull_request')
event = self._pull_request_to_event(pr_body)
+ event.account = self._get_sender(body)
event.type = 'pull_request'
if action == 'opened':
@@ -135,6 +137,7 @@
return
event = self._pull_request_to_event(pr_body)
+ event.account = self._get_sender(body)
event.comment = body.get('comment').get('body')
event.type = 'pull_request'
event.action = 'comment'
@@ -191,8 +194,43 @@
event.refspec = "refs/pull/" + str(pr_body.get('number')) + "/head"
event.patch_number = head.get('sha')
+ event.title = pr_body.get('title')
+
return event
+ def _get_sender(self, body):
+ login = body.get('sender').get('login')
+ if login:
+ return self.connection.getUser(login)
+
+
+class GithubUser(collections.Mapping):
+
+ def __init__(self, github, username):
+ self._github = github
+ self._username = username
+ self._data = None
+
+ def __getitem__(self, key):
+ if self._data is None:
+ self._data = self._init_data()
+ return self._data[key]
+
+ def __iter__(self):
+ return iter(self._data)
+
+ def __len__(self):
+ return len(self._data)
+
+ def _init_data(self):
+ user = self._github.user(self._username)
+ data = {
+ 'username': user.login,
+ 'name': user.name,
+ 'email': user.email
+ }
+ return data
+
class GithubConnection(BaseConnection):
driver_name = 'github'
@@ -250,12 +288,15 @@
change.url = event.change_url
change.updated_at = self._ghTimestampToDate(event.updated_at)
change.patchset = event.patch_number
+ change.title = event.title
+ change.source_event = event
elif event.ref:
change = Ref(project)
change.ref = event.ref
change.oldrev = event.oldrev
change.newrev = event.newrev
change.url = self.getGitwebUrl(project, sha=event.newrev)
+ change.source_event = event
else:
change = Ref(project)
return change
@@ -306,17 +347,23 @@
# For now, just send back a True value.
return True
+ def getUser(self, login):
+ return GithubUser(self.github, login)
+
+ def getUserUri(self, login):
+ return 'https://%s/%s' % (self.git_host, login)
+
def commentPull(self, project, pr_number, message):
owner, proj = project.split('/')
repository = self.github.repository(owner, proj)
pull_request = repository.issue(pr_number)
pull_request.create_comment(message)
- def mergePull(self, project, pr_number, sha=None):
+ def mergePull(self, project, pr_number, commit_message='', sha=None):
owner, proj = project.split('/')
pull_request = self.github.pull_request(owner, proj, pr_number)
try:
- result = pull_request.merge(sha=sha)
+ result = pull_request.merge(commit_message=commit_message, sha=sha)
except MethodNotAllowed as e:
raise MergeFailure('Merge was not successful due to mergeability'
' conflict, original error is %s' % e)
diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py
index 159103c..a1e8841 100644
--- a/zuul/driver/github/githubreporter.py
+++ b/zuul/driver/github/githubreporter.py
@@ -88,12 +88,13 @@
sha = item.change.patchset
self.log.debug('Reporting change %s, params %s, merging via API' %
(item.change, self.config))
+ message = self._formatMergeMessage(item.change)
try:
- self.connection.mergePull(project, pr_number, sha)
+ self.connection.mergePull(project, pr_number, message, sha)
except MergeFailure:
time.sleep(2)
self.log.debug('Trying to merge change %s again...' % item.change)
- self.connection.mergePull(project, pr_number, sha)
+ self.connection.mergePull(project, pr_number, message, sha)
item.change.is_merged = True
def setLabels(self, item):
@@ -110,6 +111,33 @@
for label in self._unlabels:
self.connection.unlabelPull(project, pr_number, label)
+ def _formatMergeMessage(self, change):
+ message = ''
+
+ if change.title:
+ message += change.title
+
+ account = change.source_event.account
+ if not account:
+ return message
+
+ username = account['username']
+ name = account['name']
+ email = account['email']
+ message += '\n\nReviewed-by: '
+
+ if name:
+ message += name
+ if email:
+ if name:
+ message += ' '
+ message += '<' + email + '>'
+ if name or email:
+ message += '\n '
+ message += self.connection.getUserUri(username)
+
+ return message
+
def getSchema():
def toList(x):
diff --git a/zuul/model.py b/zuul/model.py
index 7ed8339..db49e4e 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -1813,6 +1813,8 @@
self.status = None
self.owner = None
+ self.source_event = None
+
def _id(self):
return '%s,%s' % (self.number, self.patchset)
@@ -1863,6 +1865,7 @@
def __init__(self, project):
super(PullRequest, self).__init__(project)
self.updated_at = None
+ self.title = None
def isUpdateOf(self, other):
if (hasattr(other, 'number') and self.number == other.number and
@@ -1935,6 +1938,10 @@
class GithubTriggerEvent(TriggerEvent):
+ def __init__(self):
+ super(GithubTriggerEvent, self).__init__()
+ self.title = None
+
def isPatchsetCreated(self):
if self.type == 'pull_request':
return self.action in ['opened', 'changed']