[PATCH 2/5] docs/qapidoc: linting fixes

John Snow posted 5 patches 1 week, 3 days ago
[PATCH 2/5] docs/qapidoc: linting fixes
Posted by John Snow 1 week, 3 days ago
This restores the linting baseline in qapidoc. The order of some imports
have changed slightly due to configuring isort a little better: isort
was having difficulty understanding that "compat" and "qapidoc_legacy"
were local modules because docs/sphinx "isn't a python package".

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapi_domain.py | 25 ++++++++++++++-----------
 docs/sphinx/qapidoc.py     |  5 +++--
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/docs/sphinx/qapi_domain.py b/docs/sphinx/qapi_domain.py
index c94af5719ca..ebc46a72c61 100644
--- a/docs/sphinx/qapi_domain.py
+++ b/docs/sphinx/qapi_domain.py
@@ -20,16 +20,6 @@
 
 from docutils import nodes
 from docutils.parsers.rst import directives
-
-from compat import (
-    CompatField,
-    CompatGroupedField,
-    CompatTypedField,
-    KeywordNode,
-    ParserFix,
-    Signature,
-    SpaceNode,
-)
 from sphinx import addnodes
 from sphinx.directives import ObjectDescription
 from sphinx.domains import (
@@ -44,6 +34,16 @@
 from sphinx.util.docutils import SphinxDirective
 from sphinx.util.nodes import make_id, make_refnode
 
+from compat import (
+    CompatField,
+    CompatGroupedField,
+    CompatTypedField,
+    KeywordNode,
+    ParserFix,
+    Signature,
+    SpaceNode,
+)
+
 
 if TYPE_CHECKING:
     from typing import (
@@ -56,7 +56,6 @@
     )
 
     from docutils.nodes import Element, Node
-
     from sphinx.addnodes import desc_signature, pending_xref
     from sphinx.application import Sphinx
     from sphinx.builders import Builder
@@ -168,6 +167,8 @@ class QAPIDescription(ParserFix):
     """
 
     def handle_signature(self, sig: str, signode: desc_signature) -> Signature:
+        # pylint: disable=unused-argument
+
         # Do nothing. The return value here is the "name" of the entity
         # being documented; for QAPI, this is the same as the
         # "signature", which is just a name.
@@ -210,6 +211,8 @@ def _get_fqn(self, name: Signature) -> str:
     def add_target_and_index(
         self, name: Signature, sig: str, signode: desc_signature
     ) -> None:
+        # pylint: disable=unused-argument
+
         # name is the return value of handle_signature.
         # sig is the original, raw text argument to handle_signature.
         # For QAPI, these are identical, currently.
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 661b2c4ed0e..8011ac9efaf 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -27,6 +27,7 @@
 
 from __future__ import annotations
 
+
 __version__ = "2.0"
 
 from contextlib import contextmanager
@@ -56,8 +57,6 @@
     QAPISchemaVisitor,
 )
 from qapi.source import QAPISourceInfo
-
-from qapidoc_legacy import QAPISchemaGenRSTVisitor  # type: ignore
 from sphinx import addnodes
 from sphinx.directives.code import CodeBlock
 from sphinx.errors import ExtensionError
@@ -65,6 +64,8 @@
 from sphinx.util.docutils import SphinxDirective, switch_source_input
 from sphinx.util.nodes import nested_parse_with_titles
 
+from qapidoc_legacy import QAPISchemaGenRSTVisitor  # type: ignore
+
 
 if TYPE_CHECKING:
     from typing import (
-- 
2.48.1
Re: [PATCH 2/5] docs/qapidoc: linting fixes
Posted by Markus Armbruster 1 week ago
John Snow <jsnow@redhat.com> writes:

> This restores the linting baseline in qapidoc. The order of some imports
> have changed slightly due to configuring isort a little better: isort

Changed since when / what?

> was having difficulty understanding that "compat" and "qapidoc_legacy"
> were local modules because docs/sphinx "isn't a python package".
>
> Signed-off-by: John Snow <jsnow@redhat.com>
Re: [PATCH 2/5] docs/qapidoc: linting fixes
Posted by John Snow 6 days, 15 hours ago
On Tue, Mar 25, 2025 at 3:36 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > This restores the linting baseline in qapidoc. The order of some imports
> > have changed slightly due to configuring isort a little better: isort
>
> Changed since when / what?
>

Changed as of this patch. Before, I was running isort directly from the
docs/sphinx folder, but now I'm running it from the Python directory with
some improved configuration as I explain in this commit: by teaching isort
that compat and qapidoc_legacy are local modules, isort decides to arrange
them differently.

Imports should always go in three sections, in order:

1. Standard library imports
2. Third party imports
3. First party (local) imports

Before, it was not, because it does not understand "docs/sphinx" as a local
package with local modules - I think it gets confused because of the folder
being named "sphinx".

Really, I am just explaining why some imports get shuffled around a little
bit - the new order is "correct" and the old order was slightly wrong.


>
> > was having difficulty understanding that "compat" and "qapidoc_legacy"
> > were local modules because docs/sphinx "isn't a python package".
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
>
Re: [PATCH 2/5] docs/qapidoc: linting fixes
Posted by Markus Armbruster 6 days, 2 hours ago
John Snow <jsnow@redhat.com> writes:

> On Tue, Mar 25, 2025 at 3:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > This restores the linting baseline in qapidoc. The order of some imports
>> > have changed slightly due to configuring isort a little better: isort
>>
>> Changed since when / what?
>>
>
> Changed as of this patch. Before, I was running isort directly from the
> docs/sphinx folder, but now I'm running it from the Python directory with
> some improved configuration as I explain in this commit: by teaching isort
> that compat and qapidoc_legacy are local modules, isort decides to arrange
> them differently.
>
> Imports should always go in three sections, in order:
>
> 1. Standard library imports
> 2. Third party imports
> 3. First party (local) imports
>
> Before, it was not, because it does not understand "docs/sphinx" as a local
> package with local modules - I think it gets confused because of the folder
> being named "sphinx".
>
> Really, I am just explaining why some imports get shuffled around a little
> bit - the new order is "correct" and the old order was slightly wrong.

Please use "The order of imorts change" instead of "have changed".
Present tense makes it clear that you're talking about this patch.

>> > was having difficulty understanding that "compat" and "qapidoc_legacy"
>> > were local modules because docs/sphinx "isn't a python package".
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>