Browse Source

Move LSC check into pan project presubmit checks and check num files

git cl upload prompts a user if they upload a particularly large change,
determined by the number of CCs added on the change. With the watchlists
analyzer and eventual removal of CCing watchers from git cl, this check
needs to be updated to work without knowing the number of CCs.

This change does two things:
1. Moves the check from git cl upload to a pan-project presubmit check.
2. Update the heuristic of a LSC from > 100 CCs to > 100 files modified.

Bug: 324562917
Change-Id: I6bd0db9fde942273acebc320c8b52e81b02bd42f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5296199
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Gavin Mak 1 year ago
parent
commit
b1b1a43f07
3 changed files with 44 additions and 16 deletions
  1. 0 16
      git_cl.py
  2. 17 0
      presubmit_canned_checks.py
  3. 27 0
      tests/presubmit_unittest.py

+ 0 - 16
git_cl.py

@@ -2055,15 +2055,6 @@ class Changelist(object):
         if not options.private and not options.no_autocc and not self.GetIssue(
         if not options.private and not options.no_autocc and not self.GetIssue(
         ):
         ):
             ccs = self.GetCCList().split(',')
             ccs = self.GetCCList().split(',')
-            if len(ccs) > 100:
-                lsc = (
-                    'https://chromium.googlesource.com/chromium/src/+/HEAD/docs/'
-                    'process/lsc/lsc_workflow.md')
-                print('WARNING: This will auto-CC %s users.' % len(ccs))
-                print('LSC may be more appropriate: %s' % lsc)
-                print(
-                    'You can also use the --no-autocc flag to disable auto-CC.')
-                confirm_or_exit(action='continue')
 
 
         # Add ccs from the --cc flag.
         # Add ccs from the --cc flag.
         if options.cc:
         if options.cc:
@@ -3120,13 +3111,6 @@ class Changelist(object):
         if not options.private and not options.no_autocc and not self.GetIssue(
         if not options.private and not options.no_autocc and not self.GetIssue(
         ):
         ):
             cc = self.GetCCList().split(',')
             cc = self.GetCCList().split(',')
-        if len(cc) > 100:
-            lsc = ('https://chromium.googlesource.com/chromium/src/+/HEAD/docs/'
-                   'process/lsc/lsc_workflow.md')
-            print('WARNING: This will auto-CC %s users.' % len(cc))
-            print('LSC may be more appropriate: %s' % lsc)
-            print('You can also use the --no-autocc flag to disable auto-CC.')
-            confirm_or_exit(action='continue')
         # Add cc's from the --cc flag.
         # Add cc's from the --cc flag.
         if options.cc:
         if options.cc:
             cc.extend(options.cc)
             cc.extend(options.cc)

+ 17 - 0
presubmit_canned_checks.py

@@ -270,6 +270,20 @@ def CheckCorpLinksInFiles(input_api, output_api, source_file_filter=None):
     return []
     return []
 
 
 
 
+def CheckLargeScaleChange(input_api, output_api):
+    """Checks if the change should go through the LSC process."""
+    size = len(input_api.AffectedFiles())
+    if size <= 100:
+        return []
+    return [
+        output_api.PresubmitPromptWarning(
+            f'This change contains {size} files.\n'
+            'Consider using the LSC (large scale change) process.\n'
+            'See https://chromium.googlesource.com/chromium/src/+/HEAD/docs/process/lsc/lsc_workflow.md.'  # pylint: disable=line-too-long
+        )
+    ]
+
+
 def GetCppLintFilters(lint_filters=None):
 def GetCppLintFilters(lint_filters=None):
     filters = OFF_UNLESS_MANUALLY_ENABLED_LINT_FILTERS[:]
     filters = OFF_UNLESS_MANUALLY_ENABLED_LINT_FILTERS[:]
     if lint_filters is None:
     if lint_filters is None:
@@ -1709,6 +1723,9 @@ def PanProjectChecks(input_api,
     results.extend(
     results.extend(
         input_api.canned_checks.CheckCorpLinksInFiles(
         input_api.canned_checks.CheckCorpLinksInFiles(
             input_api, output_api, source_file_filter=sources))
             input_api, output_api, source_file_filter=sources))
+    snapshot("checking large scale change")
+    results.extend(
+        input_api.canned_checks.CheckLargeScaleChange(input_api, output_api))
 
 
     if input_api.is_committing:
     if input_api.is_committing:
         if global_checks:
         if global_checks:

+ 27 - 0
tests/presubmit_unittest.py

@@ -2035,6 +2035,33 @@ class CannedChecksUnittest(PresubmitTestsBase):
                          'chromium.git.corp.google.com', None,
                          'chromium.git.corp.google.com', None,
                          presubmit.OutputApi.PresubmitPromptWarning)
                          presubmit.OutputApi.PresubmitPromptWarning)
 
 
+    def testCannedCheckLargeScaleChange(self):
+        input_api = self.MockInputApi(
+            presubmit.Change('foo', 'foo1', self.fake_root_dir, None, 0, 0,
+                             None), False)
+        affected_files = []
+        for i in range(100):
+            affected_file = mock.MagicMock(presubmit.GitAffectedFile)
+            affected_file.LocalPath.return_value = f'foo{i}.cc'
+            affected_files.append(affected_file)
+        input_api.AffectedFiles = lambda **_: affected_files
+
+        # Don't warn if less than or equal to 100 files.
+        results = presubmit_canned_checks.CheckLargeScaleChange(
+            input_api, presubmit.OutputApi)
+        self.assertEqual(len(results), 0)
+
+        # Warn if greater than 100 files.
+        affected_files.append('bar.cc')
+
+        results = presubmit_canned_checks.CheckLargeScaleChange(
+            input_api, presubmit.OutputApi)
+        self.assertEqual(len(results), 1)
+        result = results[0]
+        self.assertEqual(result.__class__,
+                         presubmit.OutputApi.PresubmitPromptWarning)
+        self.assertIn("large scale change", result.json_format()['message'])
+
     def testCheckChangeHasNoStrayWhitespace(self):
     def testCheckChangeHasNoStrayWhitespace(self):
         self.ContentTest(
         self.ContentTest(
             lambda x, y, z: presubmit_canned_checks.
             lambda x, y, z: presubmit_canned_checks.