Simplify linting output in workflow

.github/workflows/linting.yml

@@ -53,17 +53,17 @@ jobs:
       - name: Check documentation
         id: documentation-fabric
         run: |
-          python3 tools/check_documentation.py --show-diffs modules fast blueprints
+          python3 tools/check_documentation.py --show-diffs --no-show-summary modules fast blueprints
 
       - name: Check documentation links
         id: documentation-links-fabric
         run: |
-          python3 tools/check_links.py .
+          python3 tools/check_links.py --no-show-summary .
 
       - name: Check name length (fast)
         id: name-length-fast
         run: |
-          python3 tools/check_names.py --prefix-length=10 fast/stages
+          python3 tools/check_names.py --prefix-length=10 --failed-only fast/stages
 
       - name: Check python formatting
         id: yapf
@@ -76,4 +76,4 @@ jobs:
       - name: Check blueprint metadata
         id: metadata
         run: |
-          python tools/validate_metadata.py -v blueprints
+          python tools/validate_metadata.py -v --failed-only blueprints

CONTRIBUTING.md

@@ -669,6 +669,8 @@ Options:
   --help                          Show this message and exit.
 ```
 
+As a convenience, we provide a script that runs the same set of checks as our GitHub workflow. Before submitting a PR, run `tools/lint.sh` and fix any errors. You can use the tools described above to find out more about the failures.
+
 The test workflow runs test suites in parallel. Refer to the next section for more details on running and writing tests.
 
 ## Using and writing tests

tools/check_documentation.py

@@ -72,7 +72,7 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False):
 
     diff = None
     readme = readme_path.read_text()
-    mod_name = str(readme_path.relative_to(dir_path).parent)
+    readme_rel = str(readme_path.relative_to(BASEDIR))
     current_doc = tfdoc.get_tfref_parts(readme)
     current_toc = tfdoc.get_toc_parts(readme)
     if current_doc or current_toc:
@@ -88,63 +88,63 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False):
 
       if current_doc and new_doc.content != current_doc['doc']:
         state = State.FAIL_STALE_README
-        header = f'----- {mod_name} diff -----\n'
+        header = f'----- {readme_rel} diff -----\n'
         ndiff = difflib.ndiff(current_doc['doc'].splitlines(keepends=True),
                               new_doc.content.splitlines(keepends=True))
         diff = ''.join([header] + [x for x in ndiff if x[0] != ' '])
 
       elif current_toc and new_toc != current_toc['toc']:
         state = State.FAIL_STALE_TOC
-        header = f'----- {mod_name} diff -----\n'
+        header = f'----- {readme_rel} diff -----\n'
         ndiff = difflib.ndiff(current_toc['toc'].splitlines(keepends=True),
                               new_toc.splitlines(keepends=True))
         diff = ''.join([header] + [x for x in ndiff if x[0] != ' '])
 
       elif empty := [v.name for v in newvars if not v.description]:
         state = state.FAIL_VARIABLE_DESCRIPTION
         diff = "\n".join([
-            f'----- {mod_name} variables missing description -----',
+            f'----- {readme_rel} variables missing description -----',
             ', '.join(empty),
         ])
 
       elif empty := [o.name for o in newouts if not o.description]:
         state = state.FAIL_VARIABLE_DESCRIPTION
         diff = "\n".join([
-            f'----- {mod_name} outputs missing description -----',
+            f'----- {readme_rel} outputs missing description -----',
             ', '.join(empty),
         ])
 
       elif variables != sorted(variables):
         state = state.FAIL_UNSORTED_VARS
         diff = "\n".join([
-            f'----- {mod_name} variables -----',
+            f'----- {readme_rel} variables -----',
             f'variables should be in this order: ',
             ', '.join(sorted(variables)),
         ])
 
       elif outputs != sorted(outputs):
         state = state.FAIL_UNSORTED_OUTPUTS
         diff = "\n".join([
-            f'----- {mod_name} outputs -----',
+            f'----- {readme_rel} outputs -----',
             f'outputs should be in this order: ',
             ', '.join(sorted(outputs)),
         ])
 
       elif nc := [v.name for v in newvars if not v.description.endswith('.')]:
         state = state.FAIL_VARIABLE_PERIOD
         diff = "\n".join([
-            f'----- {mod_name} variable descriptions missing ending period -----',
+            f'----- {readme_rel} variable descriptions missing ending period -----',
             ', '.join(nc),
         ])
 
       elif nc := [o.name for o in newouts if not o.description.endswith('.')]:
         state = state.FAIL_OUTPUT_PERIOD
         diff = "\n".join([
-            f'----- {mod_name} output descriptions missing ending period -----',
+            f'----- {readme_rel} output descriptions missing ending period -----',
             ', '.join(nc),
         ])
 
-      yield mod_name, state, diff
+      yield readme_rel, state, diff
 
 
 @click.command()
@@ -153,18 +153,19 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False):
 @click.option('--files/--no-files', default=False)
 @click.option('--show-diffs/--no-show-diffs', default=False)
 @click.option('--show-extra/--no-show-extra', default=False)
+@click.option('--show-summary/--no-show-summary', default=True)
 def main(dirs, exclude_file=None, files=False, show_diffs=False,
-         show_extra=False):
+         show_extra=False, show_summary=True):
   'Cycle through modules and ensure READMEs are up-to-date.'
-  print(f'files: {files}, extra: {show_extra}, diffs: {show_diffs}\n')
+  # print(f'files: {files}, extra: {show_extra}, diffs: {show_diffs}\n')
   errors = []
   for dir_name in dirs:
-    print(f'----- {dir_name} -----')
     result = _check_dir(dir_name, exclude_file, files, show_extra)
-    for mod_name, state, diff in result:
+    for readme_path, state, diff in result:
       if state.failed:
-        errors.append((mod_name, diff))
-      print(f'[{state.label}] {mod_name}')
+        errors.append((readme_path, diff))
+      if show_summary:
+        print(f'[{state.label}] {readme_path}')
 
   if errors:
     if show_diffs:
@@ -173,6 +174,7 @@ def main(dirs, exclude_file=None, files=False, show_diffs=False,
     else:
       print('Errored modules:')
       print('\n'.join([e[0] for e in errors]))
+
     raise SystemExit('Errors found.')
 
 

tools/check_links.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-# Copyright 2022 Google LLC
+# Copyright 2023 Google LLC
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -77,20 +77,24 @@ def check_docs(dir_name, external=False):
 @click.argument('dirs', type=str, nargs=-1)
 @click.option('-e', '--external', is_flag=True, default=False,
               help='Whether to test external links.')
-def main(dirs, external):
+@click.option('--show-summary/--no-show-summary', default=True)
+def main(dirs, external, show_summary=True):
   'Checks links in Markdown files contained in dirs.'
   errors = []
   for dir_name in dirs:
-    print(f'----- {dir_name} -----')
+    if show_summary:
+      print(f'----- {dir_name} -----')
     for doc in check_docs(dir_name, external):
       state = '✓' if all(l.valid for l in doc.links) else '✗'
-      print(f'[{state}] {doc.relpath} ({len(doc.links)})')
+      if show_summary:
+        print(f'[{state}] {doc.relpath} ({len(doc.links)})')
       if state == '✗':
         error = [f'{dir_name}/{doc.relpath}']
         for l in doc.links:
           if not l.valid:
             error.append(f'  - {l.dest}')
-            print(f'  {l.dest}')
+            if show_summary:
+              print(f'  {l.dest}')
         errors.append('\n'.join(error))
   if errors:
     raise SystemExit('Errors found:\n{}'.format('\n'.join(errors)))

tools/check_names.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-# Copyright 2022 Google LLC
+# Copyright 2023 Google LLC
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -72,7 +72,8 @@ def get_names(dir_name):
 @click.command()
 @click.argument('dirs', type=str, nargs=-1)
 @click.option('--prefix-length', default=7, type=int)
-def main(dirs, prefix_length=None):
+@click.option('--failed-only', is_flag=True, default=False)
+def main(dirs, prefix_length=None, failed_only=False):
   'Parse names in dirs.'
   import json
   logging.basicConfig(level=logging.INFO)
@@ -87,18 +88,21 @@ def main(dirs, prefix_length=None):
   errors = []
   for name in names:
     name_length = name.length + prefix_length
+    do_print = True
     if name_length >= MOD_LIMITS[name.source]:
       flag = "✗"
       errors += [f"{name.source}:{name.name}:{name_length}"]
     else:
       flag = "✓"
+      do_print = not failed_only
 
-    print(f"[{flag}] {name.source.ljust(source_just)} "
-          f"{name.name.ljust(name_just)} "
-          f"{name.value.ljust(value_just)} "
-          f"({name_length})")
-  if errors:
-    raise ValueError(errors)
+    if do_print:
+      print(f"[{flag}] {name.source.ljust(source_just)} "
+            f"{name.name.ljust(name_just)} "
+            f"{name.value.ljust(value_just)} "
+            f"({name_length})")
+
+  return 0 if not errors else 1
 
 
 if __name__ == '__main__':

tools/validate_metadata.py

@@ -59,7 +59,8 @@ def _validate(path: Path, validator) -> ValidationResult:
 @click.argument('dirs', type=click.Path(exists=True, file_okay=False), nargs=-1)
 @click.option('-v', '--verbose', is_flag=True, default=False,
               help='Print additional validation details.')
-def main(dirs: list[str], verbose: bool) -> int:
+@click.option('--failed-only', is_flag=True, default=False)
+def main(dirs: list[str], verbose: bool, failed_only=False) -> int:
   instances = set()
   for dir_name in dirs:
     instances |= set(Path(dir_name).glob("**/metadata.yaml"))
@@ -72,7 +73,8 @@ def main(dirs: list[str], verbose: bool) -> int:
   for instance in instances:
     result = _validate(instance, validator)
     if result.state == State.OK:
-      print(f'[✓] {instance}')
+      if not failed_only:
+        print(f'[✓] {instance}')
     else:
       print(f'[✗] {instance}')
       failed_files[instance] = result.errors