From 3c57b27ae1870bdedae3cf07e6b102e9927b5692 Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Thu, 7 Jan 2021 00:29:13 +0000 Subject: [PATCH] Revert "presubmit: Don't print comments for missing reviewers." This reverts commit 276bab4c6e0e70789eab8bc24443c587066342b4. Reason for revert: Possible cause of crbug.com/1163731 Original change's description: > presubmit: Don't print comments for missing reviewers. > > Don't print owners comments when suggesting missing reviewers, > as this won't be supported when using Gerrit API. > > Change-Id: I644b2f87b58a3c9ece3aa7d107100f3501bd3071 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2607675 > Commit-Queue: Josip Sokcevic > Reviewed-by: Gavin Mak > Reviewed-by: Josip Sokcevic > Auto-Submit: Edward Lesmes TBR=ehmaldonado@chromium.org,gavinmak@google.com,infra-scoped@luci-project-accounts.iam.gserviceaccount.com,sokcevic@google.com Bug: 1163731 Change-Id: I0b3d0f352e5483e754f6b790f70a9278df4eb3e2 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2613924 Reviewed-by: Edward Lesmes Commit-Queue: Edward Lesmes --- presubmit_canned_checks.py | 20 ++++++++++++++++++-- tests/presubmit_unittest.py | 9 +++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 123d1475d..c7ee5a0ce 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -1176,7 +1176,17 @@ def CheckOwners( owner_email = owner_email or input_api.change.author_email - missing_files = owners_db.files_not_covered_by(affected_files, reviewers) + finder = input_api.owners_finder( + affected_files, + input_api.change.RepositoryRoot(), + owner_email, + reviewers, + fopen=open, + os_path=input_api.os_path, + email_postfix='', + disable_color=True, + override_files=input_api.change.OriginalOwnersFiles()) + missing_files = finder.unreviewed_files affects_owners = any('OWNERS' in name for name in missing_files) if input_api.is_committing: @@ -1206,9 +1216,15 @@ def CheckOwners( output_list.append(output_fn('TBR for OWNERS files are ignored.')) if not input_api.is_committing: suggested_owners = owners_db.reviewers_for(missing_files, owner_email) + owners_with_comments = [] + def RecordComments(text): + owners_with_comments.append(finder.print_indent() + text) + finder.writeln = RecordComments + for owner in suggested_owners: + finder.print_comments(owner) output_list.append(output_fn('Suggested OWNERS: ' + '(Use "git-cl owners" to interactively select owners.)\n %s' % - ('\n '.join(suggested_owners)))) + ('\n '.join(owners_with_comments)))) return output_list if input_api.is_committing and not reviewers: diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index dc099fa6e..4cab06be5 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -2721,9 +2721,14 @@ the current line as well! fake_db = mock.MagicMock(owners.Database) fake_db.email_regexp = input_api.re.compile(owners.BASIC_EMAIL_REGEXP) - fake_db.files_not_covered_by.return_value = uncovered_files - input_api.owners_db = fake_db + + fake_finder = mock.MagicMock(owners_finder.OwnersFinder) + fake_finder.unreviewed_files = uncovered_files + fake_finder.print_indent = lambda: '' + # pylint: disable=unnecessary-lambda + fake_finder.print_comments = lambda owner: fake_finder.writeln(owner) + input_api.owners_finder = lambda *args, **kwargs: fake_finder input_api.is_committing = is_committing input_api.tbr = tbr input_api.dry_run = dry_run