On 9/24/19 8:28 AM, Markus Armbruster wrote: > Many error messages refer to the offending definition even though > they're preceded by an "in definition" line. Rephrase them.
This is the cleanup promised earlier in the series.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> scripts/qapi/common.py | 113 +++++++-----------
> tests/qapi-schema/alternate-array.err | 2 +-
> def check_command(expr, info):
> - name = expr['command']
> args = expr.get('data')
> boxed = expr.get('boxed', False)
>
> if boxed and args is None:
> raise QAPISemError(info, "'boxed': true requires 'data'")
> - check_type(args, info, "'data' for command '%s'" % name,
> - allow_dict=not boxed)
> - check_type(expr.get('returns'), info,
> - "'returns' for command '%s'" % name,
> - allow_array=True)
> + check_type(expr.get('data'), info, "'data'", allow_dict=not boxed)
> + check_type(expr.get('returns'), info, "'returns'", allow_array=True)
Why are you repeating expr.get('dat') here instead of reusing args? I
guess it adds consistency with the expr.get('returns') in the next line.
>
>
> def check_event(expr, info):
> - name = expr['event']
> args = expr.get('data')
> boxed = expr.get('boxed', False)
>
> if boxed and args is None:
> raise QAPISemError(info, "'boxed': true requires 'data'")
> - check_type(args, info, "'data' for event '%s'" % name,
> - allow_dict=not boxed)
> + check_type(expr.get('data'), info, "'data'", allow_dict=not boxed)
Again, why not reuse args?
> +++ b/tests/qapi-schema/args-member-case.err
> @@ -1,2 +1,2 @@
> tests/qapi-schema/args-member-case.json: In command
> 'no-way-this-will-get-whitelisted':
> -tests/qapi-schema/args-member-case.json:2: member of 'data' for command
> 'no-way-this-will-get-whitelisted' uses uppercase in name 'Arg'
> +tests/qapi-schema/args-member-case.json:2: 'data' member 'Arg' uses
> uppercase in name 'Arg'
Better, but still feels redundant for calling out 'Arg' twice. Maybe
you further clean this one later?
> +++ b/tests/qapi-schema/enum-member-case.err
> @@ -1,2 +1,2 @@
> tests/qapi-schema/enum-member-case.json: In enum
> 'NoWayThisWillGetWhitelisted':
> -tests/qapi-schema/enum-member-case.json:4: member of enum
> 'NoWayThisWillGetWhitelisted' uses uppercase in name 'Value'
> +tests/qapi-schema/enum-member-case.json:4: 'data' member uses uppercase in
> name 'Value'
Here's a similar error about uppercase that does not have the
redundancy, for comparison.
> +++ b/tests/qapi-schema/union-branch-case.err
> @@ -1,2 +1,2 @@
> tests/qapi-schema/union-branch-case.json: In union 'Uni':
> -tests/qapi-schema/union-branch-case.json:2: member of union 'Uni' uses
> uppercase in name 'Branch'
> +tests/qapi-schema/union-branch-case.json:2: 'data' member 'Branch' uses
> uppercase in name 'Branch'
Another related one.
> +++ b/tests/qapi-schema/union-optional-branch.err
> @@ -1,2 +1,2 @@
> tests/qapi-schema/union-optional-branch.json: In union 'Union':
> -tests/qapi-schema/union-optional-branch.json:2: member of union 'Union' uses
> invalid name '*a'
> +tests/qapi-schema/union-optional-branch.json:2: 'data' member '*a' uses
> invalid name '*a'
Similar type of redundancy, but this time not related to uppercase.
Overall an improvement.
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
