Merge pull request #1738 from dbungert/code-format-enforce

enforce code format with black + isort

.flake8

@@ -1,3 +1,5 @@
 [flake8]
 # gettext inserts _ into builtin path.
 builtins = _
+max-line-length = 88
+extend-ignore = E203

.git-blame-ignore-revs

@@ -0,0 +1,7 @@
+# Automatically apply to git blame with
+# `git config blame.ignorerevsfile .git-blame-ignore-revs`
+
+# inital format with black 23.x + isort 5.12.0
+34d40643ad78af72421fbdf222c29994c8852b03
+# supplemental format with black --experimental-string-processing
+c08fdab2f835d38bbf83a8310e809d673aae5557

.github/workflows/build.yaml

@@ -18,6 +18,7 @@ jobs:
     - uses: actions/checkout@v2
     - name: run
       run: sudo ./scripts/test-in-lxd.sh ${{ matrix.image }} "make check"
+
   lint:
     runs-on: ubuntu-20.04
     strategy:
@@ -31,6 +32,24 @@ jobs:
     - name: lint
       run: sudo ./scripts/test-in-lxd.sh ${{ matrix.image }} "make lint"
 
+  format-black:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v3
+      - uses: psf/black@stable
+        with:
+          version: "~= 23.0"
+          src: "console_conf subiquity subiquitycore system_setup"
+
+  format-isort:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v3
+      - uses: isort/isort-action@v1
+        with:
+          isort-version: "5.12.0"
+          sort-paths: "console_conf subiquity subiquitycore system_setup"
+
   static-typing:
     # In this job, we compare the output of mypy before and after the PR.
     if: github.event_name == 'pull_request'

.pre-commit-config.yaml

@@ -0,0 +1,11 @@
+files: "(console_conf|subiquity|subiquitycore|system_setup)"
+repos:
+  - repo: https://github.com/psf/black
+    rev: 23.7.0
+    hooks:
+      - id: black
+  - repo: https://github.com/pycqa/isort
+    rev: 5.12.0
+    hooks:
+      - id: isort
+        name: isort

CONTRIBUTING.md

@@ -4,7 +4,10 @@ see:
 
 * Contributors will need to have signed the
   [CLA](https://ubuntu.com/legal/contributors/agreement).
-* Prerequisites for running tests locally can be installed with `make aptdeps`.
-* Lint, unit, integration, and API tests should be passing.  To simulate that
-  locally please run `make lint check`.  This is a close approximation to what
-  is run in CI today.
+* Format, lint, unit, integration, and API tests should be passing.
+  * format - either run `make format` or `pre-commit install`.
+    See [pre-commit](https://pre-commit.com/#install) for more details on that
+    tool.
+  * all the rest - `make lint check`.
+    Prerequisites for running these locally can be installed with
+    `make aptdeps`.

Makefile

@@ -13,7 +13,7 @@ SYSTEM_SETUP_DRYRUN?=--dry-run
 export PYTHONPATH
 CWD := $(shell pwd)
 
-CHECK_DIRS := console_conf/ subiquity/ subiquitycore/ system_setup/
+CHECK_DIRS := console_conf subiquity subiquitycore system_setup
 PYTHON := python3
 
 ifneq (,$(MACHINE))
@@ -73,7 +73,6 @@ lint: flake8
 
 .PHONY: flake8
 flake8:
-	@echo 'tox -e flake8' is preferred to 'make flake8'
 	$(PYTHON) -m flake8 $(CHECK_DIRS)
 
 .PHONY: unit
@@ -109,6 +108,12 @@ schema: gitdeps
 	@$(PYTHON) -m subiquity.cmd.schema > autoinstall-schema.json
 	@$(PYTHON) -m system_setup.cmd.schema > autoinstall-system-setup-schema.json
 
+.PHONY: format black isort
+format:
+	env -u PYTHONPATH tox -e black,isort
+black isort:
+	env -u PYTHONPATH tox -e $@
+
 .PHONY: clean
 clean:
 	./debian/rules clean

console_conf/__init__.py

@@ -16,6 +16,7 @@
 """ Console-Conf """
 
 from subiquitycore import i18n
+
 __all__ = [
-    'i18n',
+    "i18n",
 ]

console_conf/cmd/tui.py

@@ -16,42 +16,63 @@
 
 import argparse
 import asyncio
-import sys
-import os
 import logging
-from subiquitycore.log import setup_logger
-from subiquitycore import __version__ as VERSION
+import os
+import sys
+
 from console_conf.core import ConsoleConf, RecoveryChooser
+from subiquitycore import __version__ as VERSION
+from subiquitycore.log import setup_logger
 
 
 def parse_options(argv):
     parser = argparse.ArgumentParser(
-        description=(
-            'console-conf - Pre-Ownership Configuration for Ubuntu Core'),
-        prog='console-conf')
-    parser.add_argument('--dry-run', action='store_true',
-                        dest='dry_run',
-                        help='menu-only, do not call installer function')
-    parser.add_argument('--serial', action='store_true',
-                        dest='run_on_serial',
-                        help='Run the installer over serial console.')
-    parser.add_argument('--ascii', action='store_true',
-                        dest='ascii',
-                        help='Run the installer in ascii mode.')
-    parser.add_argument('--machine-config', metavar='CONFIG',
-                        dest='machine_config',
-                        type=argparse.FileType(),
-                        help="Don't Probe. Use probe data file")
-    parser.add_argument('--screens', action='append', dest='screens',
-                        default=[])
-    parser.add_argument('--answers')
-    parser.add_argument('--recovery-chooser-mode', action='store_true',
-                        dest='chooser_systems',
-                        help=('Run as a recovery chooser interacting with the '
-                              'calling process over stdin/stdout streams'))
-    parser.add_argument('--output-base', action='store', dest='output_base',
-                        default='.subiquity',
-                        help='in dryrun, control basedir of files')
+        description="console-conf - Pre-Ownership Configuration for Ubuntu Core",
+        prog="console-conf",
+    )
+    parser.add_argument(
+        "--dry-run",
+        action="store_true",
+        dest="dry_run",
+        help="menu-only, do not call installer function",
+    )
+    parser.add_argument(
+        "--serial",
+        action="store_true",
+        dest="run_on_serial",
+        help="Run the installer over serial console.",
+    )
+    parser.add_argument(
+        "--ascii",
+        action="store_true",
+        dest="ascii",
+        help="Run the installer in ascii mode.",
+    )
+    parser.add_argument(
+        "--machine-config",
+        metavar="CONFIG",
+        dest="machine_config",
+        type=argparse.FileType(),
+        help="Don't Probe. Use probe data file",
+    )
+    parser.add_argument("--screens", action="append", dest="screens", default=[])
+    parser.add_argument("--answers")
+    parser.add_argument(
+        "--recovery-chooser-mode",
+        action="store_true",
+        dest="chooser_systems",
+        help=(
+            "Run as a recovery chooser interacting with the "
+            "calling process over stdin/stdout streams"
+        ),
+    )
+    parser.add_argument(
+        "--output-base",
+        action="store",
+        dest="output_base",
+        default=".subiquity",
+        help="in dryrun, control basedir of files",
+    )
     return parser.parse_args(argv)
 
 
@@ -64,7 +85,7 @@ def main():
     if opts.dry_run:
         LOGDIR = opts.output_base
     setup_logger(dir=LOGDIR)
-    logger = logging.getLogger('console_conf')
+    logger = logging.getLogger("console_conf")
     logger.info("Starting console-conf v{}".format(VERSION))
     logger.info("Arguments passed: {}".format(sys.argv))
 
@@ -73,8 +94,7 @@ async def run_with_loop():
             # when running as a chooser, the stdin/stdout streams are set up by
             # the process that runs us, attempt to restore the tty in/out by
             # looking at stderr
-            chooser_input, chooser_output = restore_std_streams_from(
-                    sys.stderr)
+            chooser_input, chooser_output = restore_std_streams_from(sys.stderr)
             interface = RecoveryChooser(opts, chooser_input, chooser_output)
         else:
             interface = ConsoleConf(opts)
@@ -91,10 +111,10 @@ def restore_std_streams_from(from_file):
     tty = os.ttyname(from_file.fileno())
     # we have tty now
     chooser_input, chooser_output = sys.stdin, sys.stdout
-    sys.stdin = open(tty, 'r')
-    sys.stdout = open(tty, 'w')
+    sys.stdin = open(tty, "r")
+    sys.stdout = open(tty, "w")
     return chooser_input, chooser_output
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     sys.exit(main())

console_conf/cmd/write_login_details.py

@@ -17,20 +17,18 @@
 import logging
 import sys
 
-from subiquitycore.log import setup_logger
-from subiquitycore import __version__ as VERSION
-
 from console_conf.controllers.identity import write_login_details_standalone
+from subiquitycore import __version__ as VERSION
+from subiquitycore.log import setup_logger
 
 
 def main():
-    logger = setup_logger(dir='/var/log/console-conf')
-    logger = logging.getLogger('console_conf')
-    logger.info(
-        "Starting console-conf-write-login-details v{}".format(VERSION))
+    logger = setup_logger(dir="/var/log/console-conf")
+    logger = logging.getLogger("console_conf")
+    logger.info("Starting console-conf-write-login-details v{}".format(VERSION))
     logger.info("Arguments passed: {}".format(sys.argv))
     return write_login_details_standalone()
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     sys.exit(main())

console_conf/controllers/__init__.py

@@ -15,19 +15,17 @@
 
 """ console-conf controllers """
 
-from .identity import IdentityController
 from subiquitycore.controllers.network import NetworkController
-from .welcome import WelcomeController, RecoveryChooserWelcomeController
-from .chooser import (
-    RecoveryChooserController,
-    RecoveryChooserConfirmController
-    )
+
+from .chooser import RecoveryChooserConfirmController, RecoveryChooserController
+from .identity import IdentityController
+from .welcome import RecoveryChooserWelcomeController, WelcomeController
 
 __all__ = [
-    'IdentityController',
-    'NetworkController',
-    'WelcomeController',
-    'RecoveryChooserWelcomeController',
-    'RecoveryChooserController',
-    'RecoveryChooserConfirmController',
+    "IdentityController",
+    "NetworkController",
+    "WelcomeController",
+    "RecoveryChooserWelcomeController",
+    "RecoveryChooserController",
+    "RecoveryChooserConfirmController",
 ]

console_conf/controllers/chooser.py

@@ -15,18 +15,16 @@
 import logging
 
 from console_conf.ui.views import (
-    ChooserView,
-    ChooserCurrentSystemView,
     ChooserConfirmView,
-    )
-
+    ChooserCurrentSystemView,
+    ChooserView,
+)
 from subiquitycore.tuicontroller import TuiController
 
 log = logging.getLogger("console_conf.controllers.chooser")
 
 
 class RecoveryChooserBaseController(TuiController):
-
     def __init__(self, app):
         super().__init__(app)
         self.model = app.base_model
@@ -40,7 +38,6 @@ def back(self):
 
 
 class RecoveryChooserController(RecoveryChooserBaseController):
-
     def __init__(self, app):
         super().__init__(app)
         self._model_view, self._all_view = self._make_views()
@@ -64,9 +61,9 @@ def _make_views(self):
         if self.model.current and self.model.current.actions:
             # only when we have a current system and it has actions available
             more = len(self.model.systems) > 1
-            current_view = ChooserCurrentSystemView(self,
-                                                    self.model.current,
-                                                    has_more=more)
+            current_view = ChooserCurrentSystemView(
+                self, self.model.current, has_more=more
+            )
 
         all_view = ChooserView(self, self.model.systems)
         return current_view, all_view
@@ -80,8 +77,7 @@ def more_options(self):
         self.ui.set_body(self._all_view)
 
     def back(self):
-        if self._current_view == self._all_view and \
-           self._model_view is not None:
+        if self._current_view == self._all_view and self._model_view is not None:
             # back in the all-systems screen goes back to the current model
             # screen, provided we have one
             self._current_view = self._model_view