[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 02/36] qapi: modify docstrings to be sphinx-compatible
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v5 02/36] qapi: modify docstrings to be sphinx-compatible |
Date: |
Wed, 07 Oct 2020 09:24:50 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> On 10/6/20 7:21 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>>
>>> A precise style guide and a package-wide overhaul is forthcoming pending
>>> further discussion and consensus. At present, we are avoiding obvious
>>> errors that cause sphinx documentation build problems.
>>>
>>> A preliminary style guide is loosely based on PEP 257 and Sphinx
>>> Autodoc. It is chosen for interoperability with our existing Sphinx
>>> framework, and because it has loose recognition in the Pycharm IDE.
>>>
>>> - Use Triple-double quotes (""").
>>> - Opening and closing quotes appear on their own lines for multi-line docs.
>>> - A single-sentence summary should be the first line of the docstring.
>>> - A blank line follows.
>>> - Further prose, if needed, is written next and can be multiple paragraphs,
>>> contain RST markup, etc.
>>> - The :param x: desc, :returns: desc, and :raises z: desc info fields
>>> follow.
>> Mandatory when they apply?
>>
>
> Subject of debate...
>
> - Some people really hate obvious docstring comments.
> - Some people really like the consistency.
>
> Which type of developer am I? Guess it depends on when you ask.
>
> Figured we'd hash that out when I go to write a style guide document.
Fair enough.
If I stop reading after the first paragraph, the patch matches
expectations built by the commit message.
If I speed-read, the first paragraph barely registers, but the second
makes me slow down, giving me the mistaken idea that this patch is about
converting to a preliminary style guide. It's not, it's about getting
Sphinx errors out of the way.
I figure you didn't stop after the first paragraph because you felt a
need to explain why you resolve the "obvious errors" the way you do.
Perhaps:
qapi: modify docstrings to be sphinx-compatible
A precise style guide and a package-wide overhaul is forthcoming
pending further discussion and consensus. For now, merely avoid
obvious errors that cause Sphinx documentation build problems, using a
style loosely based on PEP 257 and Sphinx Autodoc. It is chosen for
interoperability with our existing Sphinx framework, and because it
has loose recognition in the Pycharm IDE.
[...]
>>> - Additional examples, diagrams, or other metadata follows below.
>>>
>>> See also:
>>>
>>> https://www.python.org/dev/peps/pep-0257/
>>> https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#info-field-lists
Blank line here, by convention.
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>> scripts/qapi/gen.py | 6 ++++--
>>> scripts/qapi/parser.py | 1 +
>>> 2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
>>> index ca66c82b5b8..dc7b94aa115 100644
>>> --- a/scripts/qapi/gen.py
>>> +++ b/scripts/qapi/gen.py
>>> @@ -154,9 +154,11 @@ def _bottom(self):
>>> @contextmanager
>>> def ifcontext(ifcond, *args):
>>> - """A 'with' statement context manager to wrap with start_if()/end_if()
>>> + """
>>> + A with-statement context manager that wraps with `start_if()` /
>>> `end_if()`.
>>> - *args: any number of QAPIGenCCode
>>> + :param ifcond: A list of conditionals, passed to `start_if()`.
>>> + :param args: any number of `QAPIGenCCode`.
>>> Example::
>>> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>>> index 9d1a3e2eea9..31bc2e6dca9 100644
>>> --- a/scripts/qapi/parser.py
>>> +++ b/scripts/qapi/parser.py
>>> @@ -381,6 +381,7 @@ def append(self, line):
>>> The way that the line is dealt with depends on which
>>> part of
>>> the documentation we're parsing right now:
>>> +
>>> * The body section: ._append_line is ._append_body_line
>>> * An argument section: ._append_line is ._append_args_line
>>> * A features section: ._append_line is ._append_features_line
>> I'm asking because you're not adding :param line: here.
>>
>
> Yeah, it's not necessary to test the syntax of what else I've written
> with sphinx, so I didn't add it. VERY TECHNICALLY this blurb isn't
> required at all and could be deleted. You can do so if you'd like; it
> will just show up later in some other patch or series more designed to
> fix formatting.
I recommend (but do not demand) to strictly limit this commit to
"avoiding obvious errors that cause sphinx documentation build
problems."
>> Same for several other functions in this file.
>> In schema.py:
>> class QAPISchemaMember:
>> """ Represents object members, enum members and features """
>> Are the spaces next to """ okay?
>>
>
> Ideally cleaned up, but that's not a goal of this patch or series.
Got it.
- Re: [PATCH v5 03/36] qapi-gen: Separate arg-parsing from generation, (continued)
[PATCH v5 04/36] qapi: move generator entrypoint into module, John Snow, 2020/10/05
[PATCH v5 02/36] qapi: modify docstrings to be sphinx-compatible, John Snow, 2020/10/05
[PATCH v5 07/36] qapi: enforce import order/styling with isort, John Snow, 2020/10/05
[PATCH v5 05/36] qapi: Prefer explicit relative imports, John Snow, 2020/10/05
[PATCH v5 06/36] qapi: Remove wildcard includes, John Snow, 2020/10/05
[PATCH v5 09/36] qapi: add pylintrc, John Snow, 2020/10/05
[PATCH v5 08/36] qapi: delint using flake8, John Snow, 2020/10/05