On Tue, Mar 25, 2025 at 4:05 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@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 <js...@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? > Maybe! > > Hmm, according to mypy(1), strict implies warn-unused-configs. > > The question does not block this patch. > Send me a patch to drop it O:-) > > } 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. > It definitely wasn't once upon a time. It may still not be true on the oldest mypy we currently support. We don't have a clear policy for what versions of python libraries we need to support - this is a muddy, gray area. So far I just try to avoid breaking support on older versions needlessly, but I don't have an upgrade policy. If we want to integrate this directly into make check, we'll likely need to formalize this policy. > > } 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 <arm...@redhat.com> > okey-dokey, let me integrate this feedback and I'll re-send, but I'm going to wait until we hash everything else out - you had some questions on other bits in this series.
