On Wed, May 15, 2024, 1:27 PM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@redhat.com> writes: > > > On Wed, May 15, 2024 at 5:17 AM Markus Armbruster <arm...@redhat.com> > wrote: > > > >> John Snow <js...@redhat.com> writes: > >> > >> > In the coming patches, it's helpful to have a linting baseline. > However, > >> > there's no need to shuffle around the deck chairs too much, because > most > >> > of this code will be removed once the new qapidoc generator (the > >> > "transmogrifier") is in place. > >> > > >> > To ease my pain: just turn off the black auto-formatter for most, but > >> > not all, of qapidoc.py. This will help ensure that *new* code follows > a > >> > coding standard without bothering too much with cleaning up the > existing > >> > code. > >> > > >> > For manual checking for now, try "black --check qapidoc.py" from the > >> > docs/sphinx directory. "pip install black" (without root permissions) > if > >> > you do not have it installed otherwise. > >> > > >> > Signed-off-by: John Snow <js...@redhat.com> > >> > --- > >> > docs/sphinx/qapidoc.py | 16 +++++++++------- > >> > 1 file changed, 9 insertions(+), 7 deletions(-) > >> > > >> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py > >> > index f270b494f01..1655682d4c7 100644 > >> > --- a/docs/sphinx/qapidoc.py > >> > +++ b/docs/sphinx/qapidoc.py > >> > @@ -28,28 +28,30 @@ > >> > import re > >> > > >> > from docutils import nodes > >> > +from docutils.parsers.rst import Directive, directives > >> > from docutils.statemachine import ViewList > >> > -from docutils.parsers.rst import directives, Directive > >> > -from sphinx.errors import ExtensionError > >> > -from sphinx.util.nodes import nested_parse_with_titles > >> > -import sphinx > >> > -from qapi.gen import QAPISchemaVisitor > >> > from qapi.error import QAPIError, QAPISemError > >> > +from qapi.gen import QAPISchemaVisitor > >> > from qapi.schema import QAPISchema > >> > > >> > +import sphinx > >> > +from sphinx.errors import ExtensionError > >> > +from sphinx.util.nodes import nested_parse_with_titles > >> > + > >> > >> Exchanges old pylint gripe > >> > >> docs/sphinx/qapidoc.py:45:4: C0412: Imports from package sphinx are > >> not grouped (ungrouped-imports) > >> > >> for new gripes > >> > >> docs/sphinx/qapidoc.py:37:0: C0411: third party import "import > sphinx" > >> should be placed before "from qapi.error import QAPIError, QAPISemError" > >> (wrong-import-order) > >> docs/sphinx/qapidoc.py:38:0: C0411: third party import "from > >> sphinx.errors import ExtensionError" should be placed before "from > >> qapi.error import QAPIError, QAPISemError" (wrong-import-order) > >> docs/sphinx/qapidoc.py:39:0: C0411: third party import "from > >> sphinx.util.nodes import nested_parse_with_titles" should be placed > before > >> "from qapi.error import QAPIError, QAPISemError" (wrong-import-order) > >> > >> Easy enough to fix. > >> > > > > I believe these errors are caused by the fact that the tools are confused > > about the "sphinx" namespace - some interpret them as being the local > > "module", the docs/sphinx/ directory, and others believe them to be the > > third party external package. > > > > I have not been using pylint on docs/sphinx/ files because of the > > difficulty of managing imports - this environment is generally beyond the > > reaches of my python borgcube and at present I don't have plans to > > integrate it. > > > > At the moment, I am using black, isort and flake8 for qapidoc.py and > > they're happy with it. I am not using mypy because I never did the typing > > boogaloo with qapidoc.py and I won't be bothering - except for any new > code > > I write, which *will* bother. By the end of the new transmogrifier, > > qapidoc.py *will* strictly typecheck. > > > > pylint may prove to be an issue with the imports, though. isort also > seems > > to misunderstand "sphinx, the stuff in this folder" and "sphinx, the > stuff > > in a third party package" and so I'm afraid I don't have any good ability > > to get pylint to play along, here. > > > > Pleading for "Sorry, this sucks and I can't figure out how to solve it > > quickly". Maybe a future project, apologies. > > Is this pain we inflict on ourselves by naming the directory "sphinx"? > More or less, yeah. If you check the file from a CWD where there is no "sphinx" directory, it behaves more normally. Just not worth renaming it and futzing about for now. However, I did get an invocation that lets me get a clean pylint run by abusing PYTHONPATH again, so I have at least one standard baseline we can count on. I updated the do-not-merge patch to include the special magick incantations. Maybe in the future I'll make a qemu.plugins submodule instead, but that's for quite a bit later. > >> > > >> > # Sphinx up to 1.6 uses AutodocReporter; 1.7 and later > >> > # use switch_source_input. Check borrowed from kerneldoc.py. > >> > -Use_SSI = sphinx.__version__[:3] >= '1.7' > >> > +Use_SSI = sphinx.__version__[:3] >= "1.7" > >> > if Use_SSI: > >> > from sphinx.util.docutils import switch_source_input > >> > else: > >> > from sphinx.ext.autodoc import AutodocReporter > >> > > >> > > >> > -__version__ = '1.0' > >> > +__version__ = "1.0" > >> > > >> > > >> > +# fmt: off > >> > >> I figure this tells black to keep quiet for the remainder of the file. > >> Worth a comment, I think. > >> > >> > # Function borrowed from pydash, which is under the MIT license > >> > def intersperse(iterable, separator): > >> > """Yield the members of *iterable* interspersed with > *separator*.""" > >> > >> With my comments addressed > >> Reviewed-by: Markus Armbruster <arm...@redhat.com> > >> > > > > ^ Dropping this unless you're okay with the weird import orders owing to > > the strange import paradigm in the sphinx folder.r > > Feel free to keep it. > >
