Initial blackening support.

Include advice on when and how often to blacken files.

Signed-off-by: Neil Williams <neil.williams@linaro.org>

4 files changed, 76 insertions, 20 deletions

diff --git a/doc/v2/development.rst b/doc/v2/development.rst
index 21862fc6f..4913b86f5 100644
--- a/doc/v2/development.rst
+++ b/doc/v2/development.rst
@@ -83,6 +83,49 @@ Make your changes
 
   * Use comments in the code in preference to detailed commit messages.
 
+.. index:: codestyle, black
+
+.. _running_black:
+
+Blackening the source code
+--------------------------
+
+LAVA is adopting ``black`` as the formatter for LAVA source code, in a
+gradual manner. ``black`` is only available for Debian Buster and later
+- it is not suitable for backporting to Stretch. However, Buster is
+starting the release freeze process ready for a release in Q2 2019. If
+you are running Buster on your development machine already, or have
+Buster packages available to you, you can install ``black`` using
+``apt`` as usual. You can also choose to install ``black`` from
+https://github.com/ambv/black if it is not available for your OS.
+
+Initially, black is to only be applied to:
+
+* New files added via merge requests on
+  https://git.lavasoftware.org/lava/lava/
+
+* Existing files which are substantially refactored in a merge request
+  on https://git.lavasoftware.org/lava/lava/
+
+* Unit test files and files in ``./share/``
+
+When changing files with black:
+
+#. Avoid changing a file with ``black`` if the file is already part of
+   an existing merge request. Formatting changes must be secondary to
+   functional changes.
+#. The objective is not to run ``black`` on lots of files at once, that
+   just leads to large and noisy merge requests and churn. Change at
+   most two or three files at a time. (This advice can be relaxed for
+   unit test files as simply running the unit tests in the CI is
+   sufficient to show that the formatting change has not affected
+   functionality. Even then, avoid changing more than a few hundred
+   lines per merge request.)
+#. When changing files with ``black``, add the filename to the tracking
+   file in ``share/black.yaml``. In time, this will be used to keep
+   those formatting changes when the file is touched again with a merge
+   request.
+
 .. index:: developer: adding unit tests
 
 .. _developer_adding_unit_tests:
diff --git a/share/black.yaml b/share/black.yaml
new file mode 100644
index 000000000..51ca02369
--- /dev/null
+++ b/share/black.yaml
@@ -0,0 +1,3 @@
+share:
+    check-devices.py
+    validate_devices.py
diff --git a/share/check-devices.py b/share/check-devices.py
index 96a8e65ea..31502775e 100755
--- a/share/check-devices.py
+++ b/share/check-devices.py
@@ -37,11 +37,19 @@ import yaml
 
 
 def main():
-    parser = argparse.ArgumentParser(description='Check device templates')
-    parser.add_argument("--device-types", required=True, type=str,
-                        help="Path to the directory containing the device-type jinja2 templates.")
-    parser.add_argument("--devices", required=True, type=str,
-                        help="Path to directory containing the device dictionary files.")
+    parser = argparse.ArgumentParser(description="Check device templates")
+    parser.add_argument(
+        "--device-types",
+        required=True,
+        type=str,
+        help="Path to the directory containing the device-type jinja2 templates.",
+    )
+    parser.add_argument(
+        "--devices",
+        required=True,
+        type=str,
+        help="Path to directory containing the device dictionary files.",
+    )
     args = parser.parse_args()
 
     if not os.path.isdir(args.devices):
@@ -57,7 +65,8 @@ def main():
     print("Devices:")
     env = jinja2.Environment(  # nosec - YAML, not HTML, no XSS scope.
         loader=jinja2.FileSystemLoader([args.devices, args.device_types]),
-        autoescape=False)
+        autoescape=False,
+    )
 
     for device in devices:
         device_name = os.path.splitext(os.path.basename(device))[0]
@@ -72,7 +81,10 @@ def main():
             print('* %s (ERROR): redering error "%s"' % (device_name, exc))
             errors = True
         except jinja2.exceptions.TemplateSyntaxError as exc:
-            print('* %s (ERROR): invalid syntax "%s" in "%s"' % (device_name, exc, exc.filename))
+            print(
+                '* %s (ERROR): invalid syntax "%s" in "%s"'
+                % (device_name, exc, exc.filename)
+            )
             errors = True
         else:
             print("* %s" % device_name)
@@ -80,5 +92,5 @@ def main():
     return errors
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     sys.exit(main())
diff --git a/share/validate_devices.py b/share/validate_devices.py
index 92949dc0b..99610c90f 100755
--- a/share/validate_devices.py
+++ b/share/validate_devices.py
@@ -34,24 +34,22 @@ import xmlrpc.client
 
 
 def main():
-    parser = argparse.ArgumentParser(description='LAVA Dispatcher template helper')
+    parser = argparse.ArgumentParser(description="LAVA Dispatcher template helper")
     parser.add_argument(
-        '--instance',
-        type=str,
-        required=True,
-        help='Name of the instance to check')
+        "--instance", type=str, required=True, help="Name of the instance to check"
+    )
     parser.add_argument(
-        '--hostname',
+        "--hostname",
         default=None,
         type=str,
-        help='Device to check (all pipeline devices if not used)')
+        help="Device to check (all pipeline devices if not used)",
+    )
     parser.add_argument(
-        '--https',
-        action='store_true',
-        help='Use https instead of http')
+        "--https", action="store_true", help="Use https instead of http"
+    )
     args = parser.parse_args()
 
-    protocol = 'https' if args.https else 'http'
+    protocol = "https" if args.https else "http"
 
     connection = xmlrpc.client.ServerProxy("%s://%s//RPC2" % (protocol, args.instance))
     if args.hostname:
@@ -60,5 +58,5 @@ def main():
         print(connection.scheduler.validate_pipeline_devices())
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     main()