From a3762a907c027cebe689598d4eed2953dfba4614 Mon Sep 17 00:00:00 2001 From: Andrii Shyshkalov Date: Wed, 25 Nov 2020 10:20:42 +0000 Subject: [PATCH] git cl comments: shorten long URLs if possible. R=ehmaldonado Before: https://chromium-review.googlesource.com/c/2552792/3/cv/internal/gerrit/gerritfake/fake.go#321 After: https://crrev.com/c/2552792/3/cv/internal/gerrit/gerritfake/fake.go#321 Change-Id: Ie6044e2743c4359bc30c98d8915edd9119d4a386 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2556837 Auto-Submit: Andrii Shyshkalov Reviewed-by: Edward Lesmes Commit-Queue: Andrii Shyshkalov --- git_cl.py | 17 +++- tests/git_cl_test.py | 212 ++++++++++++++++++++++--------------------- 2 files changed, 122 insertions(+), 107 deletions(-) diff --git a/git_cl.py b/git_cl.py index b9a3567fc..b38dd74a1 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1785,6 +1785,15 @@ class Changelist(object): # {author+date: {path: {patchset: {line: url+message}}}} comments = collections.defaultdict( lambda: collections.defaultdict(lambda: collections.defaultdict(dict))) + + server = self.GetCodereviewServer() + if server in _KNOWN_GERRIT_TO_SHORT_URLS: + # /c/ is automatically added by short URL server. + url_prefix = '%s/%s' % (_KNOWN_GERRIT_TO_SHORT_URLS[server], + self.GetIssue()) + else: + url_prefix = '%s/c/%s' % (server, self.GetIssue()) + for path, line_comments in file_comments.items(): for comment in line_comments: tag = comment.get('tag', '') @@ -1796,10 +1805,10 @@ class Changelist(object): else: patchset = 'PS%d' % comment['patch_set'] line = comment.get('line', 0) - url = ('https://%s/c/%s/%s/%s#%s%s' % - (self.GetGerritHost(), self.GetIssue(), comment['patch_set'], path, - 'b' if comment.get('side') == 'PARENT' else '', - str(line) if line else '')) + url = ('%s/%s/%s#%s%s' % + (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']) summaries = [] diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 8b144e830..6ec82b3f7 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2443,84 +2443,86 @@ class TestGitCl(unittest.TestCase): ] } self.calls = [ - (('GetChangeComments', 'chromium-review.googlesource.com', - 'infra%2Finfra~1'), { - '/COMMIT_MSG': [ - { - 'author': {'email': u'reviewer@example.com'}, - 'updated': u'2017-03-17 05:19:37.500000000', - 'patch_set': 2, - 'side': 'REVISION', - 'message': 'Please include a bug link', - }, - ], - 'codereview.settings': [ - { - 'author': {'email': u'owner@example.com'}, - 'updated': u'2017-03-16 20:00:41.000000000', - 'patch_set': 2, - 'side': 'PARENT', - 'line': 42, - 'message': 'I removed this because it is bad', - }, - ] - }), - (('GetChangeRobotComments', 'chromium-review.googlesource.com', - 'infra%2Finfra~1'), {}), - ] * 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://chromium-review.googlesource.com/' + - u'c/1/2/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' - }, { - 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://chromium-review.googlesource' + - u'.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' - } - ]), '') - ] + (('GetChangeComments', 'chromium-review.googlesource.com', + 'infra%2Finfra~1'), { + '/COMMIT_MSG': [ + { + 'author': { + 'email': u'reviewer@example.com' + }, + 'updated': u'2017-03-17 05:19:37.500000000', + 'patch_set': 2, + 'side': 'REVISION', + 'message': 'Please include a bug link', + }, + ], + 'codereview.settings': [ + { + 'author': { + 'email': u'owner@example.com' + }, + 'updated': u'2017-03-16 20:00:41.000000000', + 'patch_set': 2, + 'side': 'PARENT', + 'line': 42, + 'message': 'I removed this because it is bad', + }, + ] + }), + (('GetChangeRobotComments', 'chromium-review.googlesource.com', + 'infra%2Finfra~1'), {}), + ] * 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' + }, { + 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' + }]), '')] expected_comments_summary = [ - git_cl._CommentSummary( - message=( - u'PTAL\n' + - u'\n' + - u'codereview.settings\n' + - u' Base, Line 42: https://chromium-review.googlesource.com/' + - u'c/1/2/codereview.settings#b42\n' + - u' I removed this because it is bad\n'), - 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://chromium-review.googlesource.com/' + - u'c/1/2//COMMIT_MSG#\n' + + 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' I removed this because it is bad\n'), + 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'), + 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') @@ -2533,7 +2535,7 @@ class TestGitCl(unittest.TestCase): # of autogenerated comment), and unlike other types of comments, only robot # comments from the latest patchset are shown. self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/infra/infra') + 'https://x.googlesource.com/infra/infra') gerrit_util.GetChangeDetail.return_value = { 'owner': {'email': 'owner@example.com'}, 'current_revision': 'ba5eba11', @@ -2597,34 +2599,38 @@ class TestGitCl(unittest.TestCase): ] } self.calls = [ - (('GetChangeComments', 'chromium-review.googlesource.com', - 'infra%2Finfra~1'), {}), - (('GetChangeRobotComments', 'chromium-review.googlesource.com', - 'infra%2Finfra~1'), { - 'codereview.settings': [ - { - u'author': {u'email': u'tricium@serviceaccount.com'}, - u'updated': u'2017-03-17 05:30:37.000000000', - u'robot_run_id': u'5565031076855808', - u'robot_id': u'Linter/Category', - u'tag': u'autogenerated:tricium', - u'patch_set': 2, - u'side': u'REVISION', - u'message': u'Linter warning message text', - u'line': 32, - }, - ], - }), + (('GetChangeComments', 'x-review.googlesource.com', 'infra%2Finfra~1'), + {}), + (('GetChangeRobotComments', 'x-review.googlesource.com', + 'infra%2Finfra~1'), { + 'codereview.settings': [ + { + u'author': { + u'email': u'tricium@serviceaccount.com' + }, + u'updated': u'2017-03-17 05:30:37.000000000', + u'robot_run_id': u'5565031076855808', + u'robot_id': u'Linter/Category', + u'tag': u'autogenerated:tricium', + u'patch_set': 2, + u'side': u'REVISION', + u'message': u'Linter warning message text', + u'line': 32, + }, + ], + }), ] expected_comments_summary = [ - git_cl._CommentSummary(date=datetime.datetime(2017, 3, 17, 5, 30, 37), - message=( - u'(1 comment)\n\ncodereview.settings\n' - u' PS2, Line 32: https://chromium-review.googlesource.com/' - u'c/1/2/codereview.settings#32\n' - u' Linter warning message text\n'), - sender=u'tricium@serviceaccount.com', - autogenerated=True, approval=False, disapproval=False) + git_cl._CommentSummary( + date=datetime.datetime(2017, 3, 17, 5, 30, 37), + message=(u'(1 comment)\n\ncodereview.settings\n' + u' PS2, Line 32: https://x-review.googlesource.com/c/1/2/' + u'codereview.settings#32\n' + u' Linter warning message text\n'), + sender=u'tricium@serviceaccount.com', + autogenerated=True, + approval=False, + disapproval=False) ] cl = git_cl.Changelist( issue=1, branchref='refs/heads/foo')