From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D4952C3064D for ; Fri, 28 Jun 2024 15:44:59 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sNDly-0006CN-D5; Fri, 28 Jun 2024 11:44:26 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sNDlv-0006Bt-SA for qemu-devel@nongnu.org; Fri, 28 Jun 2024 11:44:23 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sNDlt-0002bu-3p for qemu-devel@nongnu.org; Fri, 28 Jun 2024 11:44:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1719589460; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=/AmSu9OmMoiQ80Mn8tEVefJ2XLJpJ0VEDuT2ZqhYLFs=; b=TkvL031LqADa4jDpSN3HOZ3YwQGoY5xM9l/28A4OxGLGh/4cUKRkLLUO4ub/xLxH94Pqew ZArEWqXiur5DMlbUnxqHHzbWtQZyP9G1Siy3gpYXHYM2XxvXH4Bh2ruBHLYLpc5/ze2r76 yx/xgpcMwkEvC5OpMTmLCqtsIGCPSKE= Received: from mail-pj1-f72.google.com (mail-pj1-f72.google.com [209.85.216.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-462-7Iz8OPqoP9aDIdZ0Ei1iBg-1; Fri, 28 Jun 2024 11:44:18 -0400 X-MC-Unique: 7Iz8OPqoP9aDIdZ0Ei1iBg-1 Received: by mail-pj1-f72.google.com with SMTP id 98e67ed59e1d1-2c924a5eda9so678202a91.0 for ; Fri, 28 Jun 2024 08:44:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719589457; x=1720194257; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=/AmSu9OmMoiQ80Mn8tEVefJ2XLJpJ0VEDuT2ZqhYLFs=; b=srDJlJczEjeW9XY+0IuV9s/4AJTB0VzvWJQYooaPdRkG5g6h2Un7AnFdznoapFasqK hGO2OVfy2aUfRXC9Hp/q5/FVHX7V2HrD6fq7BNunI4dkP/mKRGKFHALkfEaNLnSK4bb6 ClyTRWGc8kRXyUC7GUmbKdtgIkeeecFx5F9Sqn08M6xhsfpCWAP6P443o9EhmJt8xdrs XJzrfVlpI7WyY+ceITE3ZkKOJZ2xbGIz2JaXhBmn3YYnjuIPMrw5/lp0uK3viUmNfDZ7 Tjv1/SR289i0qDwvlrLpFPAfbHcgxnnVdB+f8YAIpIrRYaYOSZzxnTiefwoGUvT2egjO D3LQ== X-Gm-Message-State: AOJu0YxfTrrMGOpM2I+mZzH/XtjM46jVOn3XV6Yjeo6Yn9w0scKatRkc orSTzQvkgZwPWqiM+JmslGpuVWUPDk+OsVe9CLnfcRWPuyip1F3y8NbUoOh0LlVTu8dJgyxSdPK Dx9cbfGbfyci1o1vyGrYgBuMMewJkrQCvxgXXh7Pp5qOIW6hQAPC5adD9jLNT5o0/2HFeBkziEN 3vB3w+IbWIyIwxSy8eauoIRbZb0fk= X-Received: by 2002:a17:90a:7c06:b0:2c2:fe3d:3453 with SMTP id 98e67ed59e1d1-2c86124c9f7mr13797119a91.18.1719589457372; Fri, 28 Jun 2024 08:44:17 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHud+BcI2ZFVeItfEw5ILyZ924wkgJIAzv126OvJeNr8zz4Y5LSQXISmHR3OQzOFNG2WMN/1iFwuF5X0Zim2ac= X-Received: by 2002:a17:90a:7c06:b0:2c2:fe3d:3453 with SMTP id 98e67ed59e1d1-2c86124c9f7mr13797095a91.18.1719589456912; Fri, 28 Jun 2024 08:44:16 -0700 (PDT) MIME-Version: 1.0 References: <20240626222128.406106-1-jsnow@redhat.com> <20240626222128.406106-11-jsnow@redhat.com> <87a5j5z96v.fsf@pond.sub.org> In-Reply-To: <87a5j5z96v.fsf@pond.sub.org> From: John Snow Date: Fri, 28 Jun 2024 11:44:05 -0400 Message-ID: Subject: Re: [PATCH v2 10/21] qapi: convert "Note" sections to plain rST To: Markus Armbruster Cc: qemu-devel , Mads Ynddal , Jiri Pirko , Stefan Hajnoczi , Eric Blake , Peter Maydell , Michael Roth , "Michael S. Tsirkin" , Alex Williamson , Pavel Dovgalyuk , Victor Toso de Carvalho , =?UTF-8?Q?C=C3=A9dric_Le_Goater?= , =?UTF-8?Q?Daniel_P=2E_Berrang=C3=A9?= , Qemu-block , Ani Sinha , Fabiano Rosas , Marcel Apfelbaum , =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= , Gerd Hoffmann , Paolo Bonzini , Kevin Wolf , Peter Xu , Eduardo Habkost , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Lukas Straub , Igor Mammedov , Jason Wang , Yanan Wang , Hanna Reitz , Konstantin Kostiuk Content-Type: multipart/alternative; boundary="000000000000bd449e061bf51e03" Received-SPF: pass client-ip=170.10.133.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -22 X-Spam_score: -2.3 X-Spam_bar: -- X-Spam_report: (-2.3 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.206, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --000000000000bd449e061bf51e03 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Jun 28, 2024, 5:52=E2=80=AFAM Markus Armbruster = wrote: > John Snow writes: > > > We do not need a dedicated section for notes. By eliminating a speciall= y > > 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 o= f > > the conversion, capitalize the first letter of each sentence and add > > trailing punctuation where appropriate to ensure notes look sensible an= d > > 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 goo= d > > 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); al= l > > are adorned with a (!) symbol (=EF=81=AA) in the title bar for rendered= HTML > > docs. > > > > See > > > https://sphinx-rtd-theme.readthedocs.io/en/stable/demo/demo.html#admoniti= ons > > 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 no= t > > 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 o= f > > 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 > > Acked-by: Stefan Hajnoczi [for block*.json] > > I dislike the indentation changes, and may revert them in my tree. > =F0=9F=98=A2 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 > > --000000000000bd449e061bf51e03 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Fri, Jun 28, 2024, 5:52=E2=80=AFAM Markus Armbruste= r <armbru@redhat.com> wrote:=
John Snow <jsnow@redhat.com&g= t; writes:

> We do not need a dedicated section for notes. By eliminating a special= ly
> 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 t= o 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 a= nd
> consistent in rendered HTML documentation. Markup is also re-aligned t= o
> 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 sug= gesting a
> new syntax. The exact formatting to use is a matter of taste, but a go= od
> candidate is simply:
>
> .. note:: lorem ipsum ...
>=C2=A0 =C2=A0 ... dolor sit amet ...
>=C2=A0 =C2=A0 ... consectetur adipiscing elit ...
>
> ... but there are other choices, too. The Sphinx readthedocs theme
> offers theming for the following forms (capitalization unimportant); a= ll
> are adorned with a (!) symbol (=EF=81=AA) in the title bar for rendere= d 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
>
>=C2=A0 =C2=A0 admonition body text
>
> This patch uses ".. note::" almost everywhere, with just two= "caution"
> directives. Several instances of "Notes:" have been converte= d to merely
> ".. note::" where appropriate, but ".. admonition:: not= es" is used in a
> few places where we had an ordered list of multiple notes that would n= ot
> make sense as standalone/separate admonitions.

I looked for hunks that don't 1:1 replace "Note:" or "No= tes:" by
".. note::."=C2=A0 Findings:

* Two hunks replace by ".. caution::" instead.=C2=A0 Commit messa= ge got it.
=C2=A0 Good.

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

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

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


* Two hunks drop "Note:", changing it into paragraph.=C2=A0 The p= aragraph
=C2=A0 merges into the preceding "Example" section.=C2=A0 Good, b= ut should be
=C2=A0 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 conversi= on 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 sak= e in review, I think it'd be helpful to squash them together on merge f= or the same exact reason.)

Let's just amend the commit message.


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

Perhaps tweak the paragraph above:

=C2=A0 This patch uses ".. note::" almost everywhere, with just t= wo "caution"
=C2=A0 directives. Several instances of "Notes:" have been conver= ted to
=C2=A0 merely ".. note::", or multiple ".. note::" wher= e appropriate.
=C2=A0 ".. admonition:: notes" is used in a few places where we h= ad an
=C2=A0 ordered list of multiple notes that would not make sense as
=C2=A0 standalone/separate admonitions.=C2=A0 Two "Note:" followi= ng "Example:"
=C2=A0 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<= br> > 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.

=F0=9F=98= =A2

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

<= div class=3D"gmail_quote">

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

--000000000000bd449e061bf51e03--