| Message ID | 20201005195158.2348217-3-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt1 | expand |
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? > - 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 > 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. 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?
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. >> - 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 >> 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. > 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. --js
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.
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. >
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
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. - 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 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(-)