On 10/7/20 3:24 AM, Markus Armbruster wrote:
> 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.
>
Wonder why my script didn't do that. Eh.
>>>> 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."
>
OK, I'll drop this bit for now, but I will keep the new annotations for
ifcontext, because... well. Why do it twice.
>>> 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.
>