Warn only if non-inclusive term is used
This fixes the issue where presubmit_canned_checks issues a warning if
optional arguments in CheckAuthorizedAuthor are not used at all.
This relands commit b09f2bb2f282727b3316c30659e2998ac4f1a373 with some
additional changes.
R=apolito@google.com
Bug: 1098560
Change-Id: If323d90ab7d6bcca68ed89142ea67edc4be057d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2373216
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index d0cc1ba..7c68cab 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -91,8 +91,8 @@
input_api,
output_api,
'tests',
- allowlist=test_to_run_list,
- blocklist=tests_to_skip_list,
+ files_to_check=test_to_run_list,
+ files_to_skip=tests_to_skip_list,
run_on_python3=run_on_python3))
# Validate CIPD manifests.
diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index 59a3238..f4fe28b 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -7,6 +7,7 @@
from __future__ import print_function
import os as _os
+from warnings import warn
_HERE = _os.path.dirname(_os.path.abspath(__file__))
# These filters will be disabled if callers do not explicitly supply a
@@ -108,9 +109,12 @@
"""For non-googler/chromites committers, verify the author's email address is
in AUTHORS.
"""
- # TODO(https://crbug.com/1098560): Add warnings before removing BC.
+ # TODO(https://crbug.com/1098560): Remove non inclusive parameter names.
+ if bot_whitelist is not None:
+ warn('Use bot_allowlist in CheckAuthorizedAuthor')
if bot_allowlist is None:
bot_allowlist = bot_whitelist
+
if input_api.is_committing:
error_type = output_api.PresubmitError
else:
@@ -624,7 +628,11 @@
It's mainly a wrapper for RunUnitTests. Use allowlist and blocklist to filter
tests accordingly.
"""
- # TODO(https://crbug.com/1098560): Add warnings before removing bc.
+ # TODO(https://crbug.com/1098560): Remove non inclusive parameter names.
+ if allowlist is not None or whitelist is not None:
+ warn('Use files_to_check in GetUnitTestsInDirectory')
+ if blocklist is not None or blacklist is not None:
+ warn('Use files_to_skip in GetUnitTestsInDirectory')
if files_to_check is None:
files_to_check = allowlist or whitelist
if files_to_skip is None:
@@ -722,10 +730,12 @@
Restricts itself to only find files within the Change's source repo, not
dependencies.
"""
- # TODO(https://crbug.com/1098560): Add warnings before removing BC.
+ # TODO(https://crbug.com/1098560): Remove non inclusive parameter names.
if files_to_check is None:
+ warn('Use files_to_check in GetUnitTestsRecursively')
files_to_check = allowlist or whitelist
if files_to_skip is None:
+ warn('Use files_to_skip in GetUnitTestsRecursively')
files_to_skip = blocklist or blacklist
assert files_to_check is not None
assert files_to_skip is not None
@@ -870,6 +880,13 @@
The default files_to_check enforces looking only at *.py files.
"""
+
+ # TODO(https://crbug.com/1098560): Remove non inclusive parameter names.
+ if allow_list or white_list:
+ warn('Use files_to_check in GetPylint')
+ if block_list or black_list:
+ warn('Use files_to_skip in GetPylint')
+
files_to_check = tuple(files_to_check or allow_list or white_list or
(r'.*\.py$',))
files_to_skip = tuple(files_to_skip or block_list or black_list or
diff --git a/presubmit_support.py b/presubmit_support.py
index 53ed9c5..ee2883c 100755
--- a/presubmit_support.py
+++ b/presubmit_support.py
@@ -730,10 +730,12 @@
Note: Copy-paste this function to suit your needs or use a lambda function.
"""
- # TODO(https://crbug.com/1098560): Add warnings before removing bc.
- if files_to_check is None:
+ # TODO(https://crbug.com/1098560): Remove non inclusive parameter names.
+ if files_to_check is None and (allow_list or white_list):
+ warn('Use files_to_check in FilterSourceFile')
files_to_check = allow_list or white_list
- if files_to_skip is None:
+ if files_to_skip is None and (block_list or black_list):
+ warn('Use files_to_skip in FilterSourceFile')
files_to_skip = block_list or black_list
if files_to_check is None:
