Re: [PATCH v5 09/20] docs/sphinx: Add new qapi-doc Sphinx extension

[Peter Maydell]

On Fri, 4 Sep 2020 at 15:44, Markus Armbruster <armbru@redhat.com> wrote:
>
> $ pycodestyle docs/sphinx/qapidoc.py
> docs/sphinx/qapidoc.py:42:1: E302 expected 2 blank lines, found 1
> docs/sphinx/qapidoc.py:50:1: E302 expected 2 blank lines, found 1
> docs/sphinx/qapidoc.py:74:80: E501 line too long (80 > 79 characters)
> docs/sphinx/qapidoc.py:388:80: E501 line too long (80 > 79 characters)
> docs/sphinx/qapidoc.py:391:80: E501 line too long (80 > 79 characters)
> docs/sphinx/qapidoc.py:430:1: E302 expected 2 blank lines, found 1
> docs/sphinx/qapidoc.py:489:80: E501 line too long (80 > 79 characters)
> docs/sphinx/qapidoc.py:495:1: E302 expected 2 blank lines, found 1

All fixed.

> $ PYTHONPATH=scripts pylint docs/sphinx/qapidoc.py
> ************* Module qapidoc
> docs/sphinx/qapidoc.py:36:4: E0611: No name 'AutodocReporter' in module 
> 'sphinx.ext.autodoc' (no-name-in-module)
> docs/sphinx/qapidoc.py:45:10: R1708: Do not raise StopIteration in generator, 
> use return statement instead (stop-iteration-return)
> docs/sphinx/qapidoc.py:104:4: R0201: Method could be a function (no-self-use)
> docs/sphinx/qapidoc.py:253:4: R0201: Method could be a function (no-self-use)
> docs/sphinx/qapidoc.py:34:4: C0412: Imports from package sphinx are not 
> grouped (ungrouped-imports)

Not fixed: I disagree with the linter on all these.

The first and fifth of these are unfixable because they are the
result of code that is trying to adapt to multiple versions of
Sphinx (one of which has AutodocReporter and the other of which
does not).

The second makes no sense and appears to me to be a linter
bug, because the code doesn't (directly) raise StopIteration.
In any case the function being complained about is just a
straight borrowing from pydash.

The third and fourth would mean that two of the 10 or so
_nodes_for_whatever methods would become functions and
gratuitously different in call signature from the rest
just because it happens that the current implementation
doesn't need 'self'.

(The version of pylint I have also warns about:
 * comments that say "TODO", both of which are ones that
   are carried over from the texinfo generator about dropping
   fallback code when undocumented members are outlawed
 * methods that are part of the QAPISchemaVisitor
   interface and apparently have "too many arguments")
 * single-letter variables
 * the Use_SSI variable name which is from the kerneldoc plugin
I'm going to ignore those too.)

thanks
-- PMM