diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 3ddb04face..978094fdf7 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -47,8 +47,8 @@ * @esp-idf-codeowners/other /.* @esp-idf-codeowners/tools -/.gitlab/ci/ @esp-idf-codeowners/ci /.gitlab-ci.yml @esp-idf-codeowners/ci +/.gitlab/ci/ @esp-idf-codeowners/ci /.pre-commit-config.yaml @esp-idf-codeowners/ci /.readthedocs.yml @esp-idf-codeowners/docs /CMakeLists.txt @esp-idf-codeowners/build-config @@ -150,8 +150,6 @@ /docs/**/api-reference/system/ @esp-idf-codeowners/system /docs/**/security/ @esp-idf-codeowners/security -/examples/**/*.py @esp-idf-codeowners/ci @esp-idf-codeowners/tools - /examples/bluetooth/ @esp-idf-codeowners/bluetooth /examples/build_system/ @esp-idf-codeowners/build-config /examples/common_components/ @esp-idf-codeowners/system @@ -168,6 +166,8 @@ /examples/system/ @esp-idf-codeowners/system /examples/wifi/ @esp-idf-codeowners/wifi +/examples/**/*.py @esp-idf-codeowners/ci @esp-idf-codeowners/tools + /make/ @esp-idf-codeowners/build-config /tools/ @esp-idf-codeowners/tools @@ -182,4 +182,12 @@ /tools/ldgen/ @esp-idf-codeowners/build-config /tools/mass_mfg/ @esp-idf-codeowners/app-utilities +## Note: owners here should be the same as the owners for the same example subdir, above +/tools/test_apps/build_system/ @esp-idf-codeowners/build-config +/tools/test_apps/protocols/ @esp-idf-codeowners/network @esp-idf-codeowners/app-utilities +/tools/test_apps/security/ @esp-idf-codeowners/security +/tools/test_apps/system/ @esp-idf-codeowners/system + +/tools/test_apps/**/*.py @esp-idf-codeowners/ci @esp-idf-codeowners/tools + requirements.txt @esp-idf-codeowners/tools diff --git a/tools/ci/check_codeowners.py b/tools/ci/check_codeowners.py index aa25b3cda3..b45a827341 100755 --- a/tools/ci/check_codeowners.py +++ b/tools/ci/check_codeowners.py @@ -143,10 +143,9 @@ def action_ci_check(args): add_error('no owners specified for {}'.format(path_pattern)) # Check that the file is sorted by path patterns - path_pattern_for_cmp = path_pattern.replace('-', '_') # ignore difference between _ and - for ordering - if prev_path_pattern and path_pattern_for_cmp < prev_path_pattern: - add_error('file is not sorted: {} < {}'.format(path_pattern_for_cmp, prev_path_pattern)) - prev_path_pattern = path_pattern_for_cmp + if not in_order(prev_path_pattern, path_pattern): + add_error('file is not sorted: {} < {}'.format(path_pattern, prev_path_pattern)) + prev_path_pattern = path_pattern # Check that the pattern matches at least one file files = files_by_pattern(all_files, path_pattern) @@ -167,6 +166,40 @@ def action_ci_check(args): raise SystemExit(1) +def in_order(prev, current): + """ + Return True if the ordering is correct for these two lines ('prev' should be before 'current'). + + Codeowners should be ordered alphabetically, except that order is also significant for the codeowners + syntax (the last matching line has priority). + + This means that wildcards are allowed in either order (if wildcard placed first, it's placed before a + more specific pattern as a catch-all fallback. If wildcard placed second, it's to override the match + made on a previous line i.e. '/xyz/**/*.py' to override the owner of the Python files inside /xyz/ ). + """ + if not prev: + return True # first element in file + + def is_separator(c): + return c in '-_/' # ignore differences between separators for ordering purposes + + def is_wildcard(c): + return c in '?*' + + # looping until we see a different character + for a,b in zip(prev, current): + if is_separator(a) and is_separator(b): + continue + if is_wildcard(a) or is_wildcard(b): + return True # if the strings matched up to one of them having a wildcard, treat as in order + if a != b: + return b > a + assert a == b + + # common substrings up to the common length are the same, so the longer string should be after + return len(current) >= len(prev) + + def main(): parser = argparse.ArgumentParser( sys.argv[0], description='Internal helper script for working with the CODEOWNERS file.'