Re: [PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows command

[Victor Toso]

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.

Cheers,
Victor

signature.asc
Description: PGP signature