mirror of
https://chromium.googlesource.com/chromium/tools/depot_tools.git
synced 2026-01-11 18:51:29 +00:00
git cl comments: Add --unresolved & improve --json-file
* Don't output to non-json format when --json-file is passed * Format individual comments as JSON when --json-file is used * Add "unresolved" field to comments * --unresolved flag will show only unresolved comments * Better error message when no issue is associated with the branch I intend to use this to have gemini-cli fix comments: https://chromium-review.googlesource.com/c/chromium/src/+/6944524 Change-Id: I3b4d594b065751a600661a89925c69ffbf22aed5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6944465 Reviewed-by: Scott Lee <ddoman@chromium.org> Commit-Queue: Andrew Grieve <agrieve@chromium.org>
This commit is contained in:
Andrew Grieve
63
git_cl.py
63
git_cl.py
@@ -1095,6 +1095,7 @@ _CommentSummary = collections.namedtuple(
|
||||
[
|
||||
'date',
|
||||
'message',
|
||||
'message_json',
|
||||
'sender',
|
||||
'autogenerated',
|
||||
# TODO(tandrii): these two aren't known in Gerrit.
|
||||
@@ -2590,7 +2591,7 @@ class Changelist(object):
|
||||
msg=message,
|
||||
ready=publish)
|
||||
|
||||
def GetCommentsSummary(self, readable=True):
|
||||
def GetCommentsSummary(self, readable=True, unresolved_only=False):
|
||||
# DETAILED_ACCOUNTS is to get emails in accounts.
|
||||
# CURRENT_REVISION was historically included to get the latest patchset
|
||||
# so that only the robot comments from the latest patchset were shown.
|
||||
@@ -2618,9 +2619,11 @@ class Changelist(object):
|
||||
|
||||
for path, line_comments in file_comments.items():
|
||||
for comment in line_comments:
|
||||
unresolved = comment.get('unresolved', False)
|
||||
if unresolved_only and not unresolved:
|
||||
continue
|
||||
tag = comment.get('tag', '')
|
||||
if tag.startswith(
|
||||
'autogenerated'):
|
||||
if tag.startswith('autogenerated'):
|
||||
continue
|
||||
key = (comment['author']['email'], comment['updated'])
|
||||
if comment.get('side', 'REVISION') == 'PARENT':
|
||||
@@ -2632,7 +2635,8 @@ class Changelist(object):
|
||||
(url_prefix, comment['patch_set'],
|
||||
path, 'b' if comment.get('side') == 'PARENT' else '',
|
||||
str(line) if line else ''))
|
||||
comments[key][path][patchset][line] = (url, comment['message'])
|
||||
comments[key][path][patchset][line] = (unresolved, url,
|
||||
comment['message'])
|
||||
|
||||
summaries = []
|
||||
for msg in messages:
|
||||
@@ -2659,13 +2663,18 @@ class Changelist(object):
|
||||
assert len(msg['date'].split('.')[-1]) == 9
|
||||
date = datetime.datetime.strptime(msg['date'][:-3],
|
||||
'%Y-%m-%d %H:%M:%S.%f')
|
||||
message_json = {
|
||||
'message': msg['message'],
|
||||
'comments': [],
|
||||
}
|
||||
if key in comments:
|
||||
message += '\n'
|
||||
for path, patchsets in sorted(comments.get(key, {}).items()):
|
||||
if readable:
|
||||
message += '\n%s' % path
|
||||
for patchset, lines in sorted(patchsets.items()):
|
||||
for line, (url, content) in sorted(lines.items()):
|
||||
for line, (unresolved, url, content) in sorted(lines.items()):
|
||||
resolved_str = 'unresolved' if unresolved else 'resolved'
|
||||
if line:
|
||||
line_str = 'Line %d' % line
|
||||
path_str = '%s:%d:' % (path, line)
|
||||
@@ -2673,15 +2682,24 @@ class Changelist(object):
|
||||
line_str = 'File comment'
|
||||
path_str = '%s:0:' % path
|
||||
if readable:
|
||||
message += '\n %s, %s: %s' % (patchset, line_str, url)
|
||||
message += '\n %s, %s: %s (%s)' % (patchset, line_str,
|
||||
url, resolved_str)
|
||||
message += '\n %s\n' % content
|
||||
else:
|
||||
message += '\n%s ' % path_str
|
||||
message += '\n%s (%s)' % (path_str, resolved_str)
|
||||
message += '\n%s\n' % content
|
||||
message_json['comments'].append({
|
||||
'path': path,
|
||||
'line': line,
|
||||
'patchset': patchset,
|
||||
'unresolved': unresolved,
|
||||
'content': content,
|
||||
})
|
||||
|
||||
return _CommentSummary(
|
||||
date=date,
|
||||
message=message,
|
||||
message_json=message_json,
|
||||
sender=msg['author']['email'],
|
||||
autogenerated=is_autogenerated,
|
||||
# These could be inferred from the text messages and correlated with
|
||||
@@ -4865,6 +4883,9 @@ def CMDcomments(parser, args):
|
||||
default=True,
|
||||
help='output comments in a format compatible with '
|
||||
'editor parsing')
|
||||
parser.add_option('--unresolved',
|
||||
action='store_true',
|
||||
help='Show only unresolved comments')
|
||||
parser.add_option('-j',
|
||||
'--json-file',
|
||||
help='File to write JSON summary to, or "-" for stdout')
|
||||
@@ -4879,12 +4900,30 @@ def CMDcomments(parser, args):
|
||||
|
||||
cl = Changelist(issue=issue)
|
||||
|
||||
if not cl.GetIssue():
|
||||
sys.stderr.write(
|
||||
'There is no code review associated with this branch.\n')
|
||||
return 1
|
||||
|
||||
if options.comment:
|
||||
cl.AddComment(options.comment, options.publish)
|
||||
return 0
|
||||
|
||||
summary = sorted(cl.GetCommentsSummary(readable=options.readable),
|
||||
summary = sorted(cl.GetCommentsSummary(readable=options.readable,
|
||||
unresolved_only=options.unresolved),
|
||||
key=lambda c: c.date)
|
||||
if options.json_file:
|
||||
|
||||
def pre_serialize(c):
|
||||
dct = c._asdict().copy()
|
||||
dct['date'] = dct['date'].strftime('%Y-%m-%d %H:%M:%S.%f')
|
||||
dct['message'] = dct['message_json']
|
||||
del dct['message_json']
|
||||
return dct
|
||||
|
||||
write_json(options.json_file, [pre_serialize(x) for x in summary])
|
||||
return 0
|
||||
|
||||
for comment in summary:
|
||||
if comment.disapproval:
|
||||
color = Fore.RED
|
||||
@@ -4901,14 +4940,6 @@ def CMDcomments(parser, args):
|
||||
comment.sender, Fore.RESET, '\n'.join(
|
||||
' ' + l for l in comment.message.strip().splitlines())))
|
||||
|
||||
if options.json_file:
|
||||
|
||||
def pre_serialize(c):
|
||||
dct = c._asdict().copy()
|
||||
dct['date'] = dct['date'].strftime('%Y-%m-%d %H:%M:%S.%f')
|
||||
return dct
|
||||
|
||||
write_json(options.json_file, [pre_serialize(x) for x in summary])
|
||||
return 0
|
||||
|
||||
|
||||
|
||||
@@ -3305,57 +3305,84 @@ class TestGitCl(unittest.TestCase):
|
||||
]
|
||||
}),
|
||||
] * 2 + [(('write_json', 'output.json', [{
|
||||
u'date':
|
||||
u'2017-03-16 20:00:41.000000',
|
||||
u'message': (u'PTAL\n' + u'\n' + u'codereview.settings\n' +
|
||||
u' Base, Line 42: https://crrev.com/c/1/2/'
|
||||
u'codereview.settings#b42\n' +
|
||||
u' I removed this because it is bad\n'),
|
||||
u'autogenerated':
|
||||
False,
|
||||
u'approval':
|
||||
False,
|
||||
u'disapproval':
|
||||
False,
|
||||
u'sender':
|
||||
u'owner@example.com'
|
||||
'date': '2017-03-16 20:00:41.000000',
|
||||
'message': {
|
||||
'message':
|
||||
'PTAL',
|
||||
'comments': [{
|
||||
'path': 'codereview.settings',
|
||||
'line': 42,
|
||||
'patchset': 'Base',
|
||||
'unresolved': False,
|
||||
'content': 'I removed this because it is bad'
|
||||
}]
|
||||
},
|
||||
'sender': 'owner@example.com',
|
||||
'autogenerated': False,
|
||||
'approval': False,
|
||||
'disapproval': False
|
||||
}, {
|
||||
u'date':
|
||||
u'2017-03-17 05:19:37.500000',
|
||||
u'message':
|
||||
(u'Patch Set 2: Code-Review+1\n' + u'\n' + u'/COMMIT_MSG\n' +
|
||||
u' PS2, File comment: https://crrev.com/c/1/2//COMMIT_MSG#\n' +
|
||||
u' Please include a bug link\n'),
|
||||
u'autogenerated':
|
||||
False,
|
||||
u'approval':
|
||||
False,
|
||||
u'disapproval':
|
||||
False,
|
||||
u'sender':
|
||||
u'reviewer@example.com'
|
||||
'date': '2017-03-17 05:19:37.500000',
|
||||
'message': {
|
||||
'message':
|
||||
'Patch Set 2: Code-Review+1',
|
||||
'comments': [{
|
||||
'path': '/COMMIT_MSG',
|
||||
'line': 0,
|
||||
'patchset': 'PS2',
|
||||
'unresolved': False,
|
||||
'content': 'Please include a bug link'
|
||||
}]
|
||||
},
|
||||
'sender': 'reviewer@example.com',
|
||||
'autogenerated': False,
|
||||
'approval': False,
|
||||
'disapproval': False
|
||||
}]), '')]
|
||||
expected_comments_summary = [
|
||||
git_cl._CommentSummary(
|
||||
message=(u'PTAL\n' + u'\n' + u'codereview.settings\n' +
|
||||
u' Base, Line 42: https://crrev.com/c/1/2/' +
|
||||
u'codereview.settings#b42\n' +
|
||||
u'codereview.settings#b42 (resolved)\n' +
|
||||
u' I removed this because it is bad\n'),
|
||||
message_json={
|
||||
'message':
|
||||
'PTAL',
|
||||
'comments': [{
|
||||
'path': 'codereview.settings',
|
||||
'line': 42,
|
||||
'patchset': 'Base',
|
||||
'unresolved': False,
|
||||
'content': 'I removed this because it is bad'
|
||||
}]
|
||||
},
|
||||
date=datetime.datetime(2017, 3, 16, 20, 0, 41, 0),
|
||||
autogenerated=False,
|
||||
disapproval=False,
|
||||
approval=False,
|
||||
sender=u'owner@example.com'),
|
||||
git_cl._CommentSummary(message=(
|
||||
u'Patch Set 2: Code-Review+1\n' + u'\n' + u'/COMMIT_MSG\n' +
|
||||
u' PS2, File comment: https://crrev.com/c/1/2//COMMIT_MSG#\n' +
|
||||
u' Please include a bug link\n'),
|
||||
date=datetime.datetime(
|
||||
2017, 3, 17, 5, 19, 37, 500000),
|
||||
autogenerated=False,
|
||||
disapproval=False,
|
||||
approval=False,
|
||||
sender=u'reviewer@example.com'),
|
||||
git_cl._CommentSummary(
|
||||
message=(u'Patch Set 2: Code-Review+1\n' + u'\n' +
|
||||
u'/COMMIT_MSG\n' +
|
||||
u' PS2, File comment: https://crrev.com/c/1/2/'
|
||||
u'/COMMIT_MSG# (resolved)\n' +
|
||||
u' Please include a bug link\n'),
|
||||
message_json={
|
||||
'message':
|
||||
'Patch Set 2: Code-Review+1',
|
||||
'comments': [{
|
||||
'path': '/COMMIT_MSG',
|
||||
'line': 0,
|
||||
'patchset': 'PS2',
|
||||
'unresolved': False,
|
||||
'content': 'Please include a bug link'
|
||||
}]
|
||||
},
|
||||
date=datetime.datetime(2017, 3, 17, 5, 19, 37, 500000),
|
||||
autogenerated=False,
|
||||
disapproval=False,
|
||||
approval=False,
|
||||
sender=u'reviewer@example.com'),
|
||||
]
|
||||
cl = git_cl.Changelist(issue=1, branchref='refs/heads/foo')
|
||||
self.assertEqual(cl.GetCommentsSummary(), expected_comments_summary)
|
||||
|
||||
Reference in New Issue
Block a user