Skip to content

Commit 0ba147c

Browse files
szuendDevtools-frontend LUCI CQ
authored andcommitted
Revert "Lint all files all the time"
This reverts commit e1ae94a. Reason for revert: Increases `git cl upload` time to 3+ minutes if the cache is cold. Original change's description: > Lint all files all the time > > We should lint all files in the repo to ensure that we don't get > sideeffect errors coming from EsLint TypeScript or our rules that > user Type information. > > We will pay a small price locally of around ~10 sec, compare to > the previous approach of running it only for specific files. > > Bug: 480874485 > Change-Id: I87b6760975516aea257572a0d3d25ac7d154c4ba > Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7531350 > Commit-Queue: Nikolay Vitkov <nvitkov@chromium.org> > Reviewed-by: Liviu Rau <liviurau@chromium.org> > Reviewed-by: Simon Zünd <szuend@chromium.org> Bug: 480874485 No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: If3f3090b33d99c8895ad3100f4f958e1fa35ce56 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7543693 Commit-Queue: Simon Zünd <szuend@chromium.org> Auto-Submit: Simon Zünd <szuend@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
1 parent 5a884e9 commit 0ba147c

File tree

2 files changed

+30
-11
lines changed

2 files changed

+30
-11
lines changed

PRESUBMIT.py

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ def _CheckWithNodeScript(input_api,
109109
output_api,
110110
script_path,
111111
script_arguments=None,
112+
allow_typescript=False,
112113
message=None):
113114
original_sys_path = sys.path
114115
try:
@@ -128,6 +129,23 @@ def _CheckWithNodeScript(input_api,
128129
message=message)
129130

130131

132+
def _GetFilesToLint(input_api, lint_config_files, accepted_endings):
133+
run_full_check = False
134+
files_to_lint = []
135+
136+
# We are changing the lint configuration; run the full check.
137+
if len(lint_config_files) != 0:
138+
run_full_check = True
139+
else:
140+
root_dir = input_api.os_path.join(input_api.PresubmitLocalPath())
141+
# Only run the linter on files that are relevant, to save PRESUBMIT time.
142+
files_to_lint = _GetAffectedFiles(input_api, root_dir, ['D'],
143+
accepted_endings)
144+
145+
should_bail_out = len(files_to_lint) == 0 and not run_full_check
146+
return should_bail_out, files_to_lint
147+
148+
131149
def _CheckFormat(input_api, output_api):
132150
if _IsEnvCog(input_api):
133151
return [
@@ -194,7 +212,6 @@ def CheckDevToolsLint(input_api, output_api):
194212
'scripts', 'test', 'run_lint_check.mjs')
195213

196214
lint_related_paths = [
197-
lint_path,
198215
input_api.os_path.join(input_api.PresubmitLocalPath(),
199216
'eslint.config.mjs'),
200217
input_api.os_path.join(input_api.PresubmitLocalPath(),
@@ -204,30 +221,34 @@ def CheckDevToolsLint(input_api, output_api):
204221
# This file includes the LitAnalyzer rules
205222
input_api.os_path.join(input_api.PresubmitLocalPath(), 'front_end',
206223
'tsconfig.json'),
224+
input_api.os_path.join(input_api.PresubmitLocalPath(), 'scripts',
225+
'test', 'run_lint_check.mjs'),
207226
input_api.os_path.join(input_api.PresubmitLocalPath(), 'node_modules'),
208227
]
209228

210229
lint_config_files = _GetAffectedFiles(input_api, lint_related_paths, [],
211230
[])
212-
root_dir = input_api.os_path.join(input_api.PresubmitLocalPath())
213-
lintable_files_changed = _GetAffectedFiles(input_api, root_dir, ['D'],
214-
['.css', '.mjs', '.js', '.ts'])
215-
216-
should_bail_out = len(lintable_files_changed) == 0 and len(
217-
lint_config_files) == 0
218231

232+
should_bail_out, files_to_lint = _GetFilesToLint(
233+
input_api, lint_config_files, ['.css', '.mjs', '.js', '.ts'])
219234
if should_bail_out:
220235
# Run the formatter on all non-js like files
221236
results = []
222237
results.extend(_CheckFormat(input_api, output_api))
223238
return results
224239

240+
# If there are more than 50 files to check, don't bother and check
241+
# everything, so as to not run into command line length limits on Windows.
242+
if len(files_to_lint) > 50:
243+
files_to_lint = []
244+
225245
results = []
226246
results.extend(
227247
_CheckWithNodeScript(input_api,
228248
output_api,
229249
lint_path,
230-
script_arguments=[],
250+
script_arguments=files_to_lint,
251+
allow_typescript=True,
231252
message="Lint"))
232253

233254
results.extend(_CheckFormat(input_api, output_api))

scripts/test/run_lint_check.mjs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,9 +352,7 @@ async function run() {
352352
}
353353
}
354354

355-
const frontEndFiles = scripts.filter(
356-
script => script.includes('front_end') && !script.endsWith('.test.ts'),
357-
);
355+
const frontEndFiles = scripts.filter(script => script.includes('front_end'));
358356
const esLintRules = scripts.filter(script =>
359357
script.includes('scripts/eslint_rules')
360358
);

0 commit comments

Comments
 (0)