John Snow <js...@redhat.com> writes: > On 10/6/20 7:21 AM, Markus Armbruster wrote: >> John Snow <js...@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 <js...@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.