Commit c6aa1511 authored by Josip Sokcevic's avatar Josip Sokcevic Committed by LUCI CQ

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 b09f2bb2 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/+/2373216Reviewed-by: 's avatarAnthony Polito <apolito@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
parent dff21047
...@@ -91,8 +91,8 @@ def CommonChecks(input_api, output_api, tests_to_skip_list, run_on_python3): ...@@ -91,8 +91,8 @@ def CommonChecks(input_api, output_api, tests_to_skip_list, run_on_python3):
input_api, input_api,
output_api, output_api,
'tests', 'tests',
allowlist=test_to_run_list, files_to_check=test_to_run_list,
blocklist=tests_to_skip_list, files_to_skip=tests_to_skip_list,
run_on_python3=run_on_python3)) run_on_python3=run_on_python3))
# Validate CIPD manifests. # Validate CIPD manifests.
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
from __future__ import print_function from __future__ import print_function
import os as _os import os as _os
from warnings import warn
_HERE = _os.path.dirname(_os.path.abspath(__file__)) _HERE = _os.path.dirname(_os.path.abspath(__file__))
# These filters will be disabled if callers do not explicitly supply a # These filters will be disabled if callers do not explicitly supply a
...@@ -108,9 +109,12 @@ def CheckAuthorizedAuthor(input_api, output_api, bot_allowlist=None, ...@@ -108,9 +109,12 @@ def CheckAuthorizedAuthor(input_api, output_api, bot_allowlist=None,
"""For non-googler/chromites committers, verify the author's email address is """For non-googler/chromites committers, verify the author's email address is
in AUTHORS. 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: if bot_allowlist is None:
bot_allowlist = bot_whitelist bot_allowlist = bot_whitelist
if input_api.is_committing: if input_api.is_committing:
error_type = output_api.PresubmitError error_type = output_api.PresubmitError
else: else:
...@@ -624,7 +628,11 @@ def GetUnitTestsInDirectory( ...@@ -624,7 +628,11 @@ def GetUnitTestsInDirectory(
It's mainly a wrapper for RunUnitTests. Use allowlist and blocklist to filter It's mainly a wrapper for RunUnitTests. Use allowlist and blocklist to filter
tests accordingly. 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: if files_to_check is None:
files_to_check = allowlist or whitelist files_to_check = allowlist or whitelist
if files_to_skip is None: if files_to_skip is None:
...@@ -722,10 +730,12 @@ def GetUnitTestsRecursively(input_api, output_api, directory, ...@@ -722,10 +730,12 @@ def GetUnitTestsRecursively(input_api, output_api, directory,
Restricts itself to only find files within the Change's source repo, not Restricts itself to only find files within the Change's source repo, not
dependencies. 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: if files_to_check is None:
warn('Use files_to_check in GetUnitTestsRecursively')
files_to_check = allowlist or whitelist files_to_check = allowlist or whitelist
if files_to_skip is None: if files_to_skip is None:
warn('Use files_to_skip in GetUnitTestsRecursively')
files_to_skip = blocklist or blacklist files_to_skip = blocklist or blacklist
assert files_to_check is not None assert files_to_check is not None
assert files_to_skip is not None assert files_to_skip is not None
...@@ -870,6 +880,13 @@ def GetPylint(input_api, output_api, files_to_check=None, files_to_skip=None, ...@@ -870,6 +880,13 @@ def GetPylint(input_api, output_api, files_to_check=None, files_to_skip=None,
The default files_to_check enforces looking only at *.py files. 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 files_to_check = tuple(files_to_check or allow_list or white_list or
(r'.*\.py$',)) (r'.*\.py$',))
files_to_skip = tuple(files_to_skip or block_list or black_list or files_to_skip = tuple(files_to_skip or block_list or black_list or
......
...@@ -730,10 +730,12 @@ class InputApi(object): ...@@ -730,10 +730,12 @@ class InputApi(object):
Note: Copy-paste this function to suit your needs or use a lambda function. Note: Copy-paste this function to suit your needs or use a lambda function.
""" """
# 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: 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 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 files_to_skip = block_list or black_list
if files_to_check is None: if files_to_check is None:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment