Re: [PATCH 06/15] qapi: Require member documentation (with loophole)

2024-02-12 Thread Markus Armbruster 
Daniel P. Berrangé  writes:

> On Mon, Feb 05, 2024 at 08:47:00AM +0100, Markus Armbruster wrote:
>> The QAPI generator forces you to document your stuff.  Except for
>> command arguments, event data, and members of enum and object types:
>> these the generator silently "documents" as "Not documented".
>> 
>> We can't require proper documentation there without first fixing all
>> the offenders.  We've always had too many offenders to pull that off.
>> Right now, we have more than 500.  Worse, we seem to fix old ones no
>> faster than we add new ones: in the past year, we fixed 22 ones, but
>> added 26 new ones.
>> 
>> To help arrest the backsliding, make missing documentation an error
>> unless the command, type, or event is in listed in new pragma
>> documentation-exceptions.
>
> If we want to really annoy people we could print a warning to
> stderr during docs generation, for each undocumented field.
> The many pages  of warnings would likely trigger a much quicker
> series to patches to fix it to eliminate the annoying warnings :-)

Heh.

Let's give not annoying people that much a try.  Can always escalate
later :)

[...]

Re: [PATCH 06/15] qapi: Require member documentation (with loophole)

2024-02-07 Thread Daniel P . Berrangé 
On Mon, Feb 05, 2024 at 08:47:00AM +0100, Markus Armbruster wrote:
> The QAPI generator forces you to document your stuff.  Except for
> command arguments, event data, and members of enum and object types:
> these the generator silently "documents" as "Not documented".
> 
> We can't require proper documentation there without first fixing all
> the offenders.  We've always had too many offenders to pull that off.
> Right now, we have more than 500.  Worse, we seem to fix old ones no
> faster than we add new ones: in the past year, we fixed 22 ones, but
> added 26 new ones.
> 
> To help arrest the backsliding, make missing documentation an error
> unless the command, type, or event is in listed in new pragma
> documentation-exceptions.

If we want to really annoy people we could print a warning to
stderr during docs generation, for each undocumented field.
The many pages  of warnings would likely trigger a much quicker
series to patches to fix it to eliminate the annoying warnings :-)

> 
> List all the current offenders: 117 commands and types in qapi/, and 9
> in qga/.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  docs/devel/qapi-code-gen.rst  |   5 +
>  qapi/pragma.json  | 119 ++
>  qga/qapi-schema.json  |  13 +-
>  scripts/qapi/parser.py|   7 +-
>  scripts/qapi/source.py|   2 +
>  .../qapi-schema/doc-bad-alternate-member.json |   2 +
>  tests/qapi-schema/doc-good.json   |   4 +-
>  7 files changed, 149 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[PATCH 06/15] qapi: Require member documentation (with loophole)

2024-02-04 Thread Markus Armbruster 
The QAPI generator forces you to document your stuff.  Except for
command arguments, event data, and members of enum and object types:
these the generator silently "documents" as "Not documented".

We can't require proper documentation there without first fixing all
the offenders.  We've always had too many offenders to pull that off.
Right now, we have more than 500.  Worse, we seem to fix old ones no
faster than we add new ones: in the past year, we fixed 22 ones, but
added 26 new ones.

To help arrest the backsliding, make missing documentation an error
unless the command, type, or event is in listed in new pragma
documentation-exceptions.

List all the current offenders: 117 commands and types in qapi/, and 9
in qga/.

Signed-off-by: Markus Armbruster 
---
 docs/devel/qapi-code-gen.rst  |   5 +
 qapi/pragma.json  | 119 ++
 qga/qapi-schema.json  |  13 +-
 scripts/qapi/parser.py|   7 +-
 scripts/qapi/source.py|   2 +
 .../qapi-schema/doc-bad-alternate-member.json |   2 +
 tests/qapi-schema/doc-good.json   |   4 +-
 7 files changed, 149 insertions(+), 3 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 69c8a1e8bd..756adc187e 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -167,6 +167,7 @@ Syntax::
'*doc-required': BOOL,
'*command-name-exceptions': [ STRING, ... ],
'*command-returns-exceptions': [ STRING, ... ],
+   '*documentation-exceptions': [ STRING, ... ],
'*member-name-exceptions': [ STRING, ... ] } }
 
 The pragma directive lets you control optional generator behavior.
@@ -183,6 +184,10 @@ may contain ``"_"`` instead of ``"-"``.  Default is none.
 Pragma 'command-returns-exceptions' takes a list of commands that may
 violate the rules on permitted return types.  Default is none.
 
+Pragma 'documentation-exceptions' takes a list of types, commands, and
+events whose members / arguments need not be documented.  Default is
+none.
+
 Pragma 'member-name-exceptions' takes a list of types whose member
 names may contain uppercase letters, and ``"_"`` instead of ``"-"``.
 Default is none.
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 0aa4eeddd3..0fa64742b5 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -31,6 +31,125 @@
 'query-tpm-models',
 'query-tpm-types',
 'ringbuf-read' ],
+# Types, commands, and events with undocumented members / arguments:
+'documentation-exceptions': [
+'AbortWrapper',
+'AudiodevDriver',
+'BlkdebugEvent',
+'BlockDirtyBitmapAddWrapper',
+'BlockDirtyBitmapMergeWrapper',
+'BlockDirtyBitmapWrapper',
+'BlockExportOptions',
+'BlockStatsSpecific',
+'BlockdevBackupWrapper',
+'BlockdevDriver',
+'BlockdevQcow2Encryption',
+'BlockdevQcow2EncryptionFormat',
+'BlockdevQcowEncryption',
+'BlockdevSnapshotInternalWrapper',
+'BlockdevSnapshotSyncWrapper',
+'BlockdevSnapshotWrapper',
+'BlockdevVmdkAdapterType',
+'ChardevBackend',
+'ChardevBackendKind',
+'ChardevCommonWrapper',
+'ChardevDBusWrapper',
+'ChardevFileWrapper',
+'ChardevHostdevWrapper',
+'ChardevMuxWrapper',
+'ChardevQemuVDAgentWrapper',
+'ChardevRingbufWrapper',
+'ChardevSocketWrapper',
+'ChardevSpiceChannelWrapper',
+'ChardevSpicePortWrapper',
+'ChardevStdioWrapper',
+'ChardevUdpWrapper',
+'ChardevVCWrapper',
+'CpuS390Entitlement',
+'CpuS390Polarization',
+'CpuS390State',
+'CxlCorErrorType',
+'DisplayProtocol',
+'DriveBackupWrapper',
+'DummyBlockCoreForceArrays',
+'DummyForceArrays',
+'DummyVirtioForceArrays',
+'DumpGuestMemoryCapability',
+'GrabToggleKeys',
+'GuestPanicInformationHyperV',
+'HotKeyMod',
+'HvBalloonDeviceInfoWrapper',
+'ImageInfoSpecific',
+'ImageInfoSpecificFileWrapper',
+'ImageInfoSpecificKind',
+'ImageInfoSpecificLUKSWrapper',
+'ImageInfoSpecificQCow2Wrapper',
+'ImageInfoSpecificRbdWrapper',
+'ImageInfoSpecificVmdkWrapper',
+'InetSocketAddressWrapper',
+'InputAxis',
+'InputBtnEventWrapper',
+'InputButton',
+'InputKeyEventWrapper',
+'InputMoveEventWrapper',
+'InputMultiTouchEvent',
+'InputMultiTouchEventWrapper',
+'InputMultiTouchType',
+'IntWrapper',
+'IscsiHeaderDigest',
+'IscsiTransport',
+'JSONType',
+'KeyValue',
+'KeyValueKind',
+'MemoryDeviceInfo',
+'MemoryDeviceInfoKind',
+'MigrateSetParameters',