| Message ID | 20230911104017.65397-8-victortoso@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Validate and test qapi examples | expand |
On Mon, Sep 11, 2023 at 12:40:15PM +0200, Victor Toso wrote: > 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(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel
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. Fine if the example still make sense afterwards. Do they? 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?
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
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
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',
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(-)