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

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> 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'.

As I said, use your judgement.

> (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.)

John Snow is working on a combination of pylint configuration that suits
us, genuine removal of lint, and the occasional pylint directive to shut
it up when it's wrong.  Informational, don't worry about it now.