| Message ID | 20220830161545.84198-4-victortoso@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi examples fixes and rfc for another generator | expand |
Victor Toso <victortoso@redhat.com> writes: > Example output has an extra ',' delimiter and a foreign comment > format. Fix it. > > Problem was noticed when trying to load the example into python's json > library. > > Signed-off-by: Victor Toso <victortoso@redhat.com> > --- > qapi/ui.json | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/qapi/ui.json b/qapi/ui.json > index 286c5731d1..901b84da8a 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -356,8 +356,7 @@ > # "host": "127.0.0.1", > # "channel-id": 0, > # "tls": false > -# }, > -# [ ... more channels follow ... ] > +# } > # ] > # } > # } Hmm. You're removing an ellipsis Gerd put there for a reason, I presume (commit cb42a870c3 "spice: add qmp 'query-spice' and hmp 'info spice' commands.") Even if we can do without it here, We may still want a way to abridge examples. Thoughts?
Hi, On Wed, Aug 31, 2022 at 01:50:31PM +0200, Markus Armbruster wrote: > Victor Toso <victortoso@redhat.com> writes: > > > Example output has an extra ',' delimiter and a foreign comment > > format. Fix it. > > > > Problem was noticed when trying to load the example into python's json > > library. > > > > Signed-off-by: Victor Toso <victortoso@redhat.com> > > --- > > qapi/ui.json | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/qapi/ui.json b/qapi/ui.json > > index 286c5731d1..901b84da8a 100644 > > --- a/qapi/ui.json > > +++ b/qapi/ui.json > > @@ -356,8 +356,7 @@ > > # "host": "127.0.0.1", > > # "channel-id": 0, > > # "tls": false > > -# }, > > -# [ ... more channels follow ... ] > > +# } > > # ] > > # } > > # } > > Hmm. You're removing an ellipsis Gerd put there for a reason, > I presume (commit cb42a870c3 "spice: add qmp 'query-spice' and > hmp 'info spice' commands.") I guess the reason is just that we get a too big array of SpiceChannels so the example output would be big but not interesting. > Even if we can do without it here, We may still want a way to > abridge examples. Thoughts? I just want something that can be a valid QMP message so we can transform JSON to a specific language type and then back to QMP message and compare that both matches. I don't think that, for the purpose of query-spice documentation it is worth to have a very similar but with 10 or more entries of array of channels. So, I'd say it is fine to simply cut it here. If we have a reason to put it out a 100% valid message, well, we would have that reason to back it up... but I don't have one. Cheers, Victor
Victor Toso <victortoso@redhat.com> writes: > Hi, > > On Wed, Aug 31, 2022 at 01:50:31PM +0200, Markus Armbruster wrote: >> Victor Toso <victortoso@redhat.com> writes: >> >> > Example output has an extra ',' delimiter and a foreign comment >> > format. Fix it. >> > >> > Problem was noticed when trying to load the example into python's json >> > library. >> > >> > Signed-off-by: Victor Toso <victortoso@redhat.com> >> > --- >> > qapi/ui.json | 3 +-- >> > 1 file changed, 1 insertion(+), 2 deletions(-) >> > >> > diff --git a/qapi/ui.json b/qapi/ui.json >> > index 286c5731d1..901b84da8a 100644 >> > --- a/qapi/ui.json >> > +++ b/qapi/ui.json >> > @@ -356,8 +356,7 @@ >> > # "host": "127.0.0.1", >> > # "channel-id": 0, >> > # "tls": false >> > -# }, >> > -# [ ... more channels follow ... ] >> > +# } >> > # ] >> > # } >> > # } >> >> Hmm. You're removing an ellipsis Gerd put there for a reason, >> I presume (commit cb42a870c3 "spice: add qmp 'query-spice' and >> hmp 'info spice' commands.") > > I guess the reason is just that we get a too big array of > SpiceChannels so the example output would be big but not > interesting. > >> Even if we can do without it here, We may still want a way to >> abridge examples. Thoughts? > > I just want something that can be a valid QMP message so we can > transform JSON to a specific language type and then back to QMP > message and compare that both matches. > > I don't think that, for the purpose of query-spice documentation > it is worth to have a very similar but with 10 or more entries of > array of channels. So, I'd say it is fine to simply cut it here. > If we have a reason to put it out a 100% valid message, well, we > would have that reason to back it up... but I don't have one. I agree listing more channels in the example wouldn't help users. But do we want to indicate that the example is abridged? Gerd, I'd like to have your Acked-by for this patch.
Hi, > I agree listing more channels in the example wouldn't help users. > > But do we want to indicate that the example is abridged? Hard to do if the result should be valid qmp ... Maybe add a second channel to clearly show the command returns a list of channels? I'm also curious why you check the syntax in the first place. Make sure syntax highlight in the documentation works properly? Make sure the examples are not outdated? > Gerd, I'd like to have your Acked-by for this patch. I don't mind that much. If there is a good reason go for it. Acked-by: Gerd Hoffmann <kraxel@redhat.com> take care, Gerd
diff --git a/qapi/ui.json b/qapi/ui.json index 286c5731d1..901b84da8a 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -356,8 +356,7 @@ # "host": "127.0.0.1", # "channel-id": 0, # "tls": false -# }, -# [ ... more channels follow ... ] +# } # ] # } # }
Example output has an extra ',' delimiter and a foreign comment format. Fix it. Problem was noticed when trying to load the example into python's json library. Signed-off-by: Victor Toso <victortoso@redhat.com> --- qapi/ui.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)