[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 05/14] qapi: fix example of DUMP_COMPLETED event
From: |
Victor Toso |
Subject: |
Re: [PATCH 05/14] qapi: fix example of DUMP_COMPLETED event |
Date: |
Fri, 25 Mar 2022 22:02:44 +0100 |
Hi,
On Fri, Mar 25, 2022 at 01:53:50PM +0100, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > On Thu, Mar 24, 2022 at 1:50 PM Victor Toso <victortoso@redhat.com> wrote:
> >>
> >> * Timestamp is not optional, let's add for completeness.
> >> * Add '<-' to signalize it is receiving the data
> >> * While at it, add extra space before "result" and "total"
> >>
> >> Signed-off-by: Victor Toso <victortoso@redhat.com>
> >> ---
> >> qapi/dump.json | 7 ++++---
> >> 1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/qapi/dump.json b/qapi/dump.json
> >> index f7c4267e3f..d3ed79e8cd 100644
> >> --- a/qapi/dump.json
> >> +++ b/qapi/dump.json
> >> @@ -161,9 +161,10 @@
> >> #
> >> # Example:
> >> #
> >> -# { "event": "DUMP_COMPLETED",
> >> -# "data": {"result": {"total": 1090650112, "status": "completed",
> >> -# "completed": 1090650112} } }
> >> +# <- { "event": "DUMP_COMPLETED",
> >> +# "data": { "result": { "total": 1090650112, "status": "completed",
> >> +# "completed": 1090650112} },
>
> Add a space after 1090650112, too?
ok!
> Aside: I don't actually like our use of spaces in JSON, but consistently
> ugly beats inconsistently ugly.
Ideally it would be better use some tool to pretty format/sort it
so we don't even bother looking at it too much.
> >> +# "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
> >> #
> >> ##
> >> { 'event': 'DUMP_COMPLETED' ,
> >> --
> >> 2.35.1
> >>
> >
> > Other events seem to use the timestamp as well, so go for it. I agree
> > that being able to programmatically verify docstrings is pretty
> > valuable in an API test suite.
> >
> > (What date did you choose? Does it mean anything to you? :p)
>
> Copied from some other example, I suppose.
Yes,
> I'd probably use time of writing, but that's just me.
Never thought about it. Why not, I'll change it.
> > Reviewed-by: John Snow <jsnow@redhat.com>
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Thanks!
signature.asc
Description: PGP signature