Re: [PATCH 5/5] qapi: delete un-needed python static analysis configs

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> The pylint config is being left in place because the settings differ
> enough from the python/ directory settings that we need a chit-chat on
> how to merge them O:-)
>
> Everything else can go.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/.flake8    | 3 ---
>  scripts/qapi/.isort.cfg | 7 -------
>  scripts/qapi/mypy.ini   | 4 ----
>  3 files changed, 14 deletions(-)
>  delete mode 100644 scripts/qapi/.flake8
>  delete mode 100644 scripts/qapi/.isort.cfg
>  delete mode 100644 scripts/qapi/mypy.ini
>
> diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
> deleted file mode 100644
> index a873ff67309..00000000000
> --- a/scripts/qapi/.flake8
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -[flake8]
> -# Prefer pylint's bare-except checks to flake8's
> -extend-ignore = E722

python/setup.cfg has:

   [flake8]
   # Prefer pylint's bare-except checks to flake8's
   extend-ignore = E722
   exclude = __pycache__,

Good.

> diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg
> deleted file mode 100644
> index 643caa1fbd6..00000000000
> --- a/scripts/qapi/.isort.cfg
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -[settings]
> -force_grid_wrap=4
> -force_sort_within_sections=True
> -include_trailing_comma=True
> -line_length=72
> -lines_after_imports=2
> -multi_line_output=3

python/setup.cfg has:

   [isort]
   force_grid_wrap=4
   force_sort_within_sections=True
   include_trailing_comma=True
   line_length=72
   lines_after_imports=2
   multi_line_output=3

Good.

> diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini
> deleted file mode 100644
> index 8109470a031..00000000000
> --- a/scripts/qapi/mypy.ini
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -[mypy]
> -strict = True
> -disallow_untyped_calls = False
> -python_version = 3.8

python/setup.cfg has:

   [mypy]
   strict = True
   python_version = 3.8
   warn_unused_configs = True
   namespace_packages = True
   warn_unused_ignores = False

Also a bunch of [mypy-FOO] sections that don't apply here.

You explained the differences in review of a prior iteration.  Recap:

} warn_unused_configs: Catches config values that aren't actually recognized
} or used. Was helpful once upon a time when re-arranging the Python
} directory to behave like a package to ensure that the conf files were
} working correctly.

Could this be culled now?

Hmm, according to mypy(1), strict implies warn-unused-configs.

The question does not block this patch.

} namespace_packages: Needed for the python/ directory structure (nested
} packages under a namespace, "qemu"). Doesn't impact scripts/qapi at all.
} Read up on PEP420 if you are curious. Details in commit message, see below
} if you're still curious.

mypy(1) makes me suspect this is the default.  If that's true across the
versions we care for, this could be culled.

Also does not block this patch.

} warn_unused_ignores: Needed once upon a time for cross-version mypy support
} where some versions would warn in some cases and others would not. Adding
} an ignore would effectively just invert which versions complained. Probably
} still needed, but it's hard to measure.

Harmless enough.

} python_version: Changes mypy behavior regardless of the invoking python
} interpreter to check the file as if it were to be executed by Python 3.8. I
} actually want to remove this value from setup.cfg but haven't yet. I
} removed it from the python-qemu-qmp repo and never added it for qapi.
} Removing it is actually probably correct as it will catch errors specific
} to various python versions we support, but there are some nits to iron out
} in my neck of the woods. This is a case where scripts/qapi/ is stricter
} than python/ :)
} (Not reasonable to solve for this series.)

Also present in the deleted file, so no change.

} lack of disallow_untyped_calls = False: I think this might be a remnant
} from when we gradually typed qapi; it's evidently no longer needed since
} qapi still checks fine without this affordance. The default under strict is
} True.

Fair enough.

Let's mention the differences in the commit message.  Here's my try:

    Since the previous commit, python/setup.cfg applies to scripts/qapi/
    as well.  Configuration files in scripts/qapi/ override
    python/setup.cfg.

    scripts/qapi/.flake8 and scripts/qapi/.isort.cfg actually match
    python/setup.cfg exactly, and can go.

    The differences between scripts/qapi/mypy.ini and python/setup.cfg
    are harmless: [list the differences, explain why they're harmless as
    long as you can keep it brief, and if not, fall back to "trust me"].
    So scripts/qapi/mypy.ini can go, too.

    The pylint config is being left in place because the settings differ
    enough from the python/ directory settings that we need a chit-chat on
    how to merge them O:-)

With something like that
Reviewed-by: Markus Armbruster <armbru@redhat.com>