On Tue, 29 Sep 2020 at 07:54, Markus Armbruster <arm...@redhat.com> wrote:
>
> Peter Maydell <peter.mayd...@linaro.org> writes:
>
> > Some of our documentation is auto-generated from documentation
> > comments in the JSON schema.
> >
> > For Sphinx, rather than creating a file to include, the most natural
> > way to handle this is to have a small custom Sphinx extension which
> > processes the JSON file and inserts documentation into the rST
> > file being processed.
> >
> > This is the same approach that kerneldoc and hxtool use.
> >
> > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>
> Down to a few trivial things, which I can tidy up in my tree.
Thanks, that would be great.
> > + def _serror(self, msg):
> > + """Raise an exception giving a user-friendly syntax error
> > message"""
> > + file = self._cur_doc.info.fname
> > + line = self._cur_doc.info.line
> > + raise ExtensionError(
> > + '%s line %d: syntax error: %s' % (file, line, msg))
>
> Unused, let's drop.
Oops, yes. I refactored the error-handling code below and forgot
to delete this now-unused method.
> > + def visit_enum_type(self, name, info, ifcond, features, members,
> > prefix):
> > + doc = self._cur_doc
> > + self._add_doc('Enum',
> > + self._nodes_for_enum_values(doc) +
> > + self._nodes_for_features(doc) +
> > + self._nodes_for_sections(doc) +
> > + self._nodes_for_if_section(ifcond))
>
> PEP 8: In Python code, it is permissible to break before or after a
> binary operator, as long as the convention is consistent locally. For
> new code Knuth's style is suggested.
>
> Mind switching to Knuth style, i.e. break before the operator?
Fine with me, I have no strong Python style preferences.
I'm unlikely to be able to reliably follow any particular
style rules in future code I write unless there's a linter
that complains about violations, though...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5eed1e692b4..dbddb0a7635 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3149,6 +3149,7 @@ M: Peter Maydell <peter.mayd...@linaro.org>
> > S: Maintained
> > F: docs/conf.py
> > F: docs/*/conf.py
> > +F: docs/sphinx/
> >
> > Miscellaneous
> > -------------
>
> Maintainers of scripts/qapi/ should help review patches to
> docs/sphinx/qapidoc.py. Two options:
>
> * Add docs/sphinx/qapidoc.py to section QAPI. The QAPI maintainers
> become M: of this small part of "Sphinx documentation configuration
> and build machinery". R: would be more accurate. The inaccuracy
> feels harmless.
>
> * Do nothing. scripts/get_maintainer.pl won't loop in the QAPI
> maintainers. The Sphinx documentation maintainers may have to do that
> manually.
>
> What do you think?
No strong preference. I guess putting qapidoc.py in the QAPI
section means that the right people get cc'd so that's the
better approach.
> My review is of uneven value: the QAPI-facing parts are good, the
> Sphinx-related parts merely look good to ignorant me. I guess that's
> good enough, since we also have "generated docs look sane".
>
> Preferably tidied up a bit more:
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
thanks
-- PMM
