Commit 636b13fc authored by Aaron Gable's avatar Aaron Gable Committed by Commit Bot

Let Changelist().AddComment() mark changes as Ready

In Rietveld, adding a comment to a change automatically
published it no matter what. In Gerrit, we need to explicitly
mark the change as Ready for Review. This CL adds a new
parameter to the wrapper methods around the SetReview
API so that they can mark changes as Ready.

Bug: 740950
Change-Id: Icb2ad7c5beb03a4760657a761841745f0d75514e
Reviewed-on: https://chromium-review.googlesource.com/572031Reviewed-by: 's avatarAndrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
parent 68b54e78
...@@ -707,7 +707,7 @@ def RemoveReviewers(host, change, remove=None): ...@@ -707,7 +707,7 @@ def RemoveReviewers(host, change, remove=None):
'from change %s' % (r, change)) 'from change %s' % (r, change))
def SetReview(host, change, msg=None, labels=None, notify=None): def SetReview(host, change, msg=None, labels=None, notify=None, ready=None):
"""Set labels and/or add a message to a code review.""" """Set labels and/or add a message to a code review."""
if not msg and not labels: if not msg and not labels:
return return
...@@ -719,6 +719,8 @@ def SetReview(host, change, msg=None, labels=None, notify=None): ...@@ -719,6 +719,8 @@ def SetReview(host, change, msg=None, labels=None, notify=None):
body['labels'] = labels body['labels'] = labels
if notify: if notify:
body['notify'] = 'ALL' if notify else 'NONE' body['notify'] = 'ALL' if notify else 'NONE'
if ready:
body['ready'] = True
conn = CreateHttpConn(host, path, reqtype='POST', body=body) conn = CreateHttpConn(host, path, reqtype='POST', body=body)
response = ReadHttpJsonResponse(conn) response = ReadHttpJsonResponse(conn)
if labels: if labels:
......
...@@ -1717,8 +1717,8 @@ class Changelist(object): ...@@ -1717,8 +1717,8 @@ class Changelist(object):
# Forward methods to codereview specific implementation. # Forward methods to codereview specific implementation.
def AddComment(self, message): def AddComment(self, message, publish=None):
return self._codereview_impl.AddComment(message) return self._codereview_impl.AddComment(message, publish=publish)
def GetCommentsSummary(self, readable=True): def GetCommentsSummary(self, readable=True):
"""Returns list of _CommentSummary for each comment. """Returns list of _CommentSummary for each comment.
...@@ -1823,7 +1823,7 @@ class _ChangelistCodereviewBase(object): ...@@ -1823,7 +1823,7 @@ class _ChangelistCodereviewBase(object):
"""Update the description on codereview site.""" """Update the description on codereview site."""
raise NotImplementedError() raise NotImplementedError()
def AddComment(self, message): def AddComment(self, message, publish=None):
"""Posts a comment to the codereview site.""" """Posts a comment to the codereview site."""
raise NotImplementedError() raise NotImplementedError()
...@@ -2001,7 +2001,7 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase): ...@@ -2001,7 +2001,7 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
def GetIssueOwner(self): def GetIssueOwner(self):
return (self.GetIssueProperties() or {}).get('owner_email') return (self.GetIssueProperties() or {}).get('owner_email')
def AddComment(self, message): def AddComment(self, message, publish=None):
return self.RpcServer().add_comment(self.GetIssue(), message) return self.RpcServer().add_comment(self.GetIssue(), message)
def GetCommentsSummary(self, _readable=True): def GetCommentsSummary(self, _readable=True):
...@@ -2593,9 +2593,9 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): ...@@ -2593,9 +2593,9 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
gerrit_util.SetCommitMessage(self._GetGerritHost(), self.GetIssue(), gerrit_util.SetCommitMessage(self._GetGerritHost(), self.GetIssue(),
description, notify='NONE') description, notify='NONE')
def AddComment(self, message): def AddComment(self, message, publish=None):
gerrit_util.SetReview(self._GetGerritHost(), self.GetIssue(), gerrit_util.SetReview(self._GetGerritHost(), self.GetIssue(),
msg=message) msg=message, ready=publish)
def GetCommentsSummary(self, readable=True): def GetCommentsSummary(self, readable=True):
# DETAILED_ACCOUNTS is to get emails in accounts. # DETAILED_ACCOUNTS is to get emails in accounts.
......
...@@ -3389,19 +3389,20 @@ class TestGitCl(TestCase): ...@@ -3389,19 +3389,20 @@ class TestGitCl(TestCase):
def test_git_cl_comment_add_rietveld(self): def test_git_cl_comment_add_rietveld(self):
self.mock(git_cl._RietveldChangelistImpl, 'AddComment', self.mock(git_cl._RietveldChangelistImpl, 'AddComment',
lambda _, message: self._mocked_call('AddComment', message)) lambda _, message, publish: self._mocked_call(
'AddComment', message, publish))
self.calls = [ self.calls = [
((['git', 'config', 'rietveld.autoupdate'],), CERR1), ((['git', 'config', 'rietveld.autoupdate'],), CERR1),
((['git', 'config', 'rietveld.server'],), 'codereview.chromium.org'), ((['git', 'config', 'rietveld.server'],), 'codereview.chromium.org'),
(('AddComment', 'msg'), ''), (('AddComment', 'msg', None), ''),
] ]
self.assertEqual(0, git_cl.main(['comment', '--rietveld', self.assertEqual(0, git_cl.main(['comment', '--rietveld',
'-i', '10', '-a', 'msg'])) '-i', '10', '-a', 'msg']))
def test_git_cl_comment_add_gerrit(self): def test_git_cl_comment_add_gerrit(self):
self.mock(git_cl.gerrit_util, 'SetReview', self.mock(git_cl.gerrit_util, 'SetReview',
lambda host, change, msg: lambda host, change, msg, ready:
self._mocked_call('SetReview', host, change, msg)) self._mocked_call('SetReview', host, change, msg, ready))
self.calls = [ self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), CERR1), ((['git', 'symbolic-ref', 'HEAD'],), CERR1),
((['git', 'symbolic-ref', 'HEAD'],), CERR1), ((['git', 'symbolic-ref', 'HEAD'],), CERR1),
...@@ -3410,7 +3411,8 @@ class TestGitCl(TestCase): ...@@ -3410,7 +3411,8 @@ class TestGitCl(TestCase):
'origin/master'), 'origin/master'),
((['git', 'config', 'remote.origin.url'],), ((['git', 'config', 'remote.origin.url'],),
'https://chromium.googlesource.com/infra/infra'), 'https://chromium.googlesource.com/infra/infra'),
(('SetReview', 'chromium-review.googlesource.com', 10, 'msg'), None), (('SetReview', 'chromium-review.googlesource.com', 10, 'msg', None),
None),
] ]
self.assertEqual(0, git_cl.main(['comment', '--gerrit', '-i', '10', self.assertEqual(0, git_cl.main(['comment', '--gerrit', '-i', '10',
'-a', 'msg'])) '-a', 'msg']))
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment