diff mbox series

[05/14] qapi: fix example of DUMP_COMPLETED event

Message ID 20220324175015.232794-6-victortoso@redhat.com (mailing list archive)
State New, archived
Headers show
Series Fix some qapi examples and a TODO section | expand

Commit Message

Victor Toso March 24, 2022, 5:50 p.m. UTC
* 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(-)

Comments

John Snow March 24, 2022, 8:53 p.m. UTC | #1
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} },
> +#      "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)

Reviewed-by: John Snow <jsnow@redhat.com>
Markus Armbruster March 25, 2022, 12:53 p.m. UTC | #2
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?

Aside: I don't actually like our use of spaces in JSON, but consistently
ugly beats inconsistently ugly.

>> +#      "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.  I'd probably use time of
writing, but that's just me.

> Reviewed-by: John Snow <jsnow@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Victor Toso March 25, 2022, 9:02 p.m. UTC | #3
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!
diff mbox series

Patch

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} },
+#      "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
 #
 ##
 { 'event': 'DUMP_COMPLETED' ,