Re: [PATCH v3 18/30] docstring: Fix generated documentation of virStorageVolInfoFlags
Hi, On Fri, Apr 22, 2022 at 02:13:50AM -0700, Andrea Bolognani wrote: > On Thu, Apr 21, 2022 at 08:24:39PM +0200, Victor Toso wrote: > > On Thu, Apr 21, 2022 at 03:48:51PM +0200, Peter Krempa wrote: > > > Actually, this paragraph is inaccurate since the commits were renamed. > > > > > > I'd prefer a standalone description of what's going on so since this > > > patch is okay itself I'll probably go with: > > > > > > The API xml description file generator doesn't properly handle cases > > > when there's either a single comment or mixed use of pre- and post- > > > comments explaining the values. > > > > > > Modify the comments to avoid the problem and also append version > > > information for the exposed values. > > > > Ack! > > In addition to what Peter said, changes that are fixing the existing > comments so that they are handled correctly by apibuild.py should not > be mixed with changes that are adding version numbers. The former > should happen in their own patches at the beginning of the series, > which we should be able to push right away even as the rest of the > changes are still being tweaked. > > Patch 3/30 is an example of this being done right, both when it comes > to its actual contents and its position in the series. This patch and > the next few ones are not, and should be changed to look more like > 3/30. Yes, I agree. I missed this in the previous respin. It'll be fixed in the next one. Thanks! Victor signature.asc Description: PGP signature
Re: [PATCH v3 18/30] docstring: Fix generated documentation of virStorageVolInfoFlags
On Thu, Apr 21, 2022 at 08:24:39PM +0200, Victor Toso wrote: > On Thu, Apr 21, 2022 at 03:48:51PM +0200, Peter Krempa wrote: > > Actually, this paragraph is inaccurate since the commits were renamed. > > > > I'd prefer a standalone description of what's going on so since this > > patch is okay itself I'll probably go with: > > > > The API xml description file generator doesn't properly handle cases > > when there's either a single comment or mixed use of pre- and post- > > comments explaining the values. > > > > Modify the comments to avoid the problem and also append version > > information for the exposed values. > > Ack! In addition to what Peter said, changes that are fixing the existing comments so that they are handled correctly by apibuild.py should not be mixed with changes that are adding version numbers. The former should happen in their own patches at the beginning of the series, which we should be able to push right away even as the rest of the changes are still being tweaked. Patch 3/30 is an example of this being done right, both when it comes to its actual contents and its position in the series. This patch and the next few ones are not, and should be changed to look more like 3/30. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v3 18/30] docstring: Fix generated documentation of virStorageVolInfoFlags
Hi,
On Thu, Apr 21, 2022 at 03:48:51PM +0200, Peter Krempa wrote:
> On Thu, Apr 21, 2022 at 15:43:55 +0200, Peter Krempa wrote:
> > On Wed, Apr 20, 2022 at 21:08:07 +0200, Victor Toso wrote:
> > > This commit is similar to "docs: Fix generated documentation of
> > > virConnectListAllNodeDeviceFlags", check it out for more info.
>
> Actually, this paragraph is inaccurate since the commits were renamed.
>
> I'd prefer a standalone description of what's going on so since this
> patch is okay itself I'll probably go with:
>
> The API xml description file generator doesn't properly handle cases
> when there's either a single comment or mixed use of pre- and post-
> comments explaining the values.
>
> Modify the comments to avoid the problem and also append version
> information for the exposed values.
Ack!
> > > Using git diff --word-diff to show the fixed output xml (redacted).
> > >
> > >> > [-type='virStorageVolInfoFlags'/>-]
> > > {+type='virStorageVolInfoFlags' version='3.0.0' info='Return the
> > > physical size in allocation'/>+}
> > > ...
> > >> > type='virStorageVolInfoFlags'
> > > [-info='Return the physical size in allocation'/>-]
> > > {+version='3.0.0'/>+}
>
> This is also inaccurate as version info isn't added to the XML yet.
>
> I'll probably just drop this part.
Yep, leftover of moving commits around.
> > > Signed-off-by: Victor Toso
> > > ---
> > > include/libvirt/libvirt-storage.h | 7 ++-
> > > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > Reviewed-by: Peter Krempa
Thanks!
Victor
signature.asc
Description: PGP signature
Re: [PATCH v3 18/30] docstring: Fix generated documentation of virStorageVolInfoFlags
On Thu, Apr 21, 2022 at 15:43:55 +0200, Peter Krempa wrote:
> On Wed, Apr 20, 2022 at 21:08:07 +0200, Victor Toso wrote:
> > This commit is similar to "docs: Fix generated documentation of
> > virConnectListAllNodeDeviceFlags", check it out for more info.
Actually, this paragraph is inaccurate since the commits were renamed.
I'd prefer a standalone description of what's going on so since this
patch is okay itself I'll probably go with:
The API xml description file generator doesn't properly handle cases
when there's either a single comment or mixed use of pre- and post-
comments explaining the values.
Modify the comments to avoid the problem and also append version
information for the exposed values.
> >
> > Using git diff --word-diff to show the fixed output xml (redacted).
> >
> >> [-type='virStorageVolInfoFlags'/>-]
> > {+type='virStorageVolInfoFlags' version='3.0.0' info='Return the
> > physical size in allocation'/>+}
> > ...
> >> type='virStorageVolInfoFlags'
> > [-info='Return the physical size in allocation'/>-]
> > {+version='3.0.0'/>+}
This is also inaccurate as version info isn't added to the XML yet.
I'll probably just drop this part.
> >
> > Signed-off-by: Victor Toso
> > ---
> > include/libvirt/libvirt-storage.h | 7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
>
> Reviewed-by: Peter Krempa
>
Re: [PATCH v3 18/30] docstring: Fix generated documentation of virStorageVolInfoFlags
On Wed, Apr 20, 2022 at 21:08:07 +0200, Victor Toso wrote:
> This commit is similar to "docs: Fix generated documentation of
> virConnectListAllNodeDeviceFlags", check it out for more info.
>
> Using git diff --word-diff to show the fixed output xml (redacted).
>
>[-type='virStorageVolInfoFlags'/>-]
> {+type='virStorageVolInfoFlags' version='3.0.0' info='Return the physical
> size in allocation'/>+}
> ...
>type='virStorageVolInfoFlags'
> [-info='Return the physical size in allocation'/>-]
> {+version='3.0.0'/>+}
>
> Signed-off-by: Victor Toso
> ---
> include/libvirt/libvirt-storage.h | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Peter Krempa
[PATCH v3 18/30] docstring: Fix generated documentation of virStorageVolInfoFlags
This commit is similar to "docs: Fix generated documentation of
virConnectListAllNodeDeviceFlags", check it out for more info.
Using git diff --word-diff to show the fixed output xml (redacted).
-]
{+type='virStorageVolInfoFlags' version='3.0.0' info='Return the physical
size in allocation'/>+}
...
-]
{+version='3.0.0'/>+}
Signed-off-by: Victor Toso
---
include/libvirt/libvirt-storage.h | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/libvirt/libvirt-storage.h
b/include/libvirt/libvirt-storage.h
index 434454f455..8b57b1f2e8 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -248,11 +248,8 @@ typedef enum {
*
*/
typedef enum {
-VIR_STORAGE_VOL_USE_ALLOCATION = 0,
-
-/* Return the physical size in allocation */
-VIR_STORAGE_VOL_GET_PHYSICAL = 1 << 0,
-
+VIR_STORAGE_VOL_USE_ALLOCATION = 0, /* (Since: v3.0.0) */
+VIR_STORAGE_VOL_GET_PHYSICAL = 1 << 0, /* Return the physical size in
allocation (Since: v3.0.0) */
} virStorageVolInfoFlags;
/**
--
2.35.1
