qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 10/21] qapi: convert "Note" sections to plain rST


From: John Snow
Subject: Re: [PATCH v2 10/21] qapi: convert "Note" sections to plain rST
Date: Fri, 28 Jun 2024 11:44:05 -0400



On Fri, Jun 28, 2024, 5:52 AM Markus Armbruster <armbru@redhat.com> wrote:
John Snow <jsnow@redhat.com> writes:

> We do not need a dedicated section for notes. By eliminating a specially
> parsed section, these notes can be treated as normal rST paragraphs in
> the new QMP reference manual, and can be placed and styled much more
> flexibly.
>
> Convert all existing "Note" and "Notes" sections to pure rST. As part of
> the conversion, capitalize the first letter of each sentence and add
> trailing punctuation where appropriate to ensure notes look sensible and
> consistent in rendered HTML documentation. Markup is also re-aligned to
> the de-facto standard of 3 spaces for directives.
>
> Update docs/devel/qapi-code-gen.rst to reflect the new paradigm, and
> update the QAPI parser to prohibit "Note" sections while suggesting a
> new syntax. The exact formatting to use is a matter of taste, but a good
> candidate is simply:
>
> .. note:: lorem ipsum ...
>    ... dolor sit amet ...
>    ... consectetur adipiscing elit ...
>
> ... but there are other choices, too. The Sphinx readthedocs theme
> offers theming for the following forms (capitalization unimportant); all
> are adorned with a (!) symbol () in the title bar for rendered HTML
> docs.
>
> See
> https://sphinx-rtd-theme.readthedocs.io/en/stable/demo/demo.html#admonitions
> for examples of each directive/admonition in use.
>
> These are rendered in orange:
>
> .. Attention:: ...
> .. Caution:: ...
> .. WARNING:: ...
>
> These are rendered in red:
>
> .. DANGER:: ...
> .. Error:: ...
>
> These are rendered in green:
>
> .. Hint:: ...
> .. Important:: ...
> .. Tip:: ...
>
> These are rendered in blue:
>
> .. Note:: ...
> .. admonition:: custom title
>
>    admonition body text
>
> This patch uses ".. note::" almost everywhere, with just two "caution"
> directives. Several instances of "Notes:" have been converted to merely
> ".. note::" where appropriate, but ".. admonition:: notes" is used in a
> few places where we had an ordered list of multiple notes that would not
> make sense as standalone/separate admonitions.

I looked for hunks that don't 1:1 replace "Note:" or "Notes:" by
".. note::."  Findings:

* Two hunks replace by ".. caution::" instead.  Commit message got it.
  Good.

* Four hunks replace by ".. admonition:: notes", one of them as a test.
  Commit message got it.  Good.

* Three hunks split "Notes:" into multiple ".. note::".  Good, but could
  be mentioned in commit message.

I meant to imply it when discussing when admonition was used, but yeah.


* Two hunks drop "Note:", changing it into paragraph.  The paragraph
  merges into the preceding "Example" section.  Good, but should be
  mentioned in the commit message, or turned into a separate patch.

Eh. we got enough commits. I think it's helpful to keep the whole conversion in one giant bang so that the diff is helpful in illustrating all of the different types of conversions.

(In fact, even though I split out Example conversion for your sake in review, I think it'd be helpful to squash them together on merge for the same exact reason.)

Let's just amend the commit message.


* One hunk adjusts a test case for the removal of the "Note:" tag.
  Good, but could be mentioned in the commit message.

Perhaps tweak the paragraph above:

  This patch uses ".. note::" almost everywhere, with just two "caution"
  directives. Several instances of "Notes:" have been converted to
  merely ".. note::", or multiple ".. note::" where appropriate.
  ".. admonition:: notes" is used in a few places where we had an
  ordered list of multiple notes that would not make sense as
  standalone/separate admonitions.  Two "Note:" following "Example:"
  have been turned into ordinary paragraphs within the example.

Okay?

Yep, suits me fine.


> NOTE: Because qapidoc.py does not attempt to preserve source ordering of
> sections, the conversion of Notes from a "tagged section" to an
> "untagged section" means that rendering order for some notes *may
> change* as a result of this patch. The forthcoming qapidoc.py rewrite
> strictly preserves source ordering in the rendered documentation, so
> this issue will be rectified in the new generator.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> [for block*.json]

I dislike the indentation changes, and may revert them in my tree.

😢

Would you take a patch adjusting the indent later, or will you then tell me it's not worth the git blame fuzz? :)


Reviewed-by: Markus Armbruster <armbru@redhat.com>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]