[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows comm
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows command |
Date: |
Thu, 14 Sep 2023 15:15:31 +0100 |
User-agent: |
Mutt/2.2.9 (2022-11-12) |
On Thu, Sep 14, 2023 at 04:01:55PM +0200, Victor Toso wrote:
> Hi,
>
> On Thu, Sep 14, 2023 at 03:50:23PM +0200, Markus Armbruster wrote:
> > Victor Toso <victortoso@redhat.com> writes:
> >
> > > Example output has a comment embedded in the array. Remove it.
> > > The end result is a list of size 1.
> > >
> > > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > > ---
> > > qapi/rocker.json | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/qapi/rocker.json b/qapi/rocker.json
> > > index 31ce0b36f6..858e4f4a45 100644
> > > --- a/qapi/rocker.json
> > > +++ b/qapi/rocker.json
> > > @@ -249,8 +249,7 @@
> > > # "cookie": 0,
> > > # "action": {"goto-tbl": 10},
> > > # "mask": {"in-pport": 4294901760}
> > > -# },
> > > -# {...more...},
> > > +# }
> > > # ]}
> > > ##
> > > { 'command': 'query-rocker-of-dpa-flows',
> >
> > The schema patches in this series fix typos, except for this patch and
> > the next one, which drop "more of the same omitted for brevity" text. I
> > believe you drop the text because it doesn't parse as JSON.
>
> That's correct.
>
> > Fine if the example still make sense afterwards. Do they?
>
> It depends what you mean by making sense. I did not setup rocker
> to do this query and copied a real example. I think the real
> example would have a list of size more than one.
>
> So, if you think about real examples, it might not make sense. If
> we talk about clarifying they API, I think it is reasonable.
>
> > Shortening examples is a reasonable thing to do. Perhaps we
> > should adopt a conventional way to do it, and teach the
> > proposed generator to cope with it. What do you think?
>
> Yep, I like it. In reality, I did not do this change in v1 but it
> was suggested by Daniel that the end result of introducing this
> generator would be to have it run without errors, so I shortened
> as a simple way to fix it.
>
> So, should we instead move forward with another convention for
> comments inside the examples? This way we could still have a list
> size 1 with this patch but it would be clear that the expectation
> is a bigger list.
Personally I'd say if a field is a list, then the example should
contain 2 elements, just to make it a little more obvious at a
glance, as opposed to relying on spottnig the []. But that's not
a massively strong argument.
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 v2 02/11] qapi: fix example of dumpdtb command, (continued)
- [PATCH v2 02/11] qapi: fix example of dumpdtb command, Victor Toso, 2023/09/11
- [PATCH v2 04/11] qapi: fix example of set-vcpu-dirty-limit command, Victor Toso, 2023/09/11
- [PATCH v2 05/11] qapi: fix example of calc-dirty-rate command, Victor Toso, 2023/09/11
- [PATCH v2 06/11] qapi: fix example of NETDEV_STREAM_CONNECTED event, Victor Toso, 2023/09/11
- [PATCH v2 08/11] qapi: fix example of query-spice command, Victor Toso, 2023/09/11
- [PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows command, Victor Toso, 2023/09/11
- [PATCH v2 09/11] qapi: fix example of query-blockstats command, Victor Toso, 2023/09/11
- [PATCH v2 10/11] qapi: meson: add test flag to allow skip generators, Victor Toso, 2023/09/11
- Re: [PATCH v2 00/11] Validate and test qapi examples, Markus Armbruster, 2023/09/14