Re: [PATCH v2 0/7] qapi: static typing conversion, pt5c

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> This is part five (c), and focuses on sharing strict types between
> parser.py and expr.py.
>
> gitlab: https://gitlab.com/jsnow/qemu/-/commits/python-qapi-cleanup-pt5c
>
> Every commit should pass with:
>  - `isort -c qapi/`
>  - `flake8 qapi/`
>  - `pylint --rcfile=qapi/pylintrc qapi/`
>  - `mypy --config-file=qapi/mypy.ini qapi/`
>
> Some notes on this series:
>
> Patches 2 and 3 are almost entirely superseded by patch 5, but I wasn't
> as confident that Markus would like patch 5, so these patches aren't
> squashed quite as tightly as they could be -- I recommend peeking ahead
> at the cover letters before reviewing the actual patch diffs.

Yes, you're taking a somewhat roundabout path there.

I think I like PATCH 5 well enough.  Do you have a tighter squash in
mind?

> By the end of this series, the only JSON-y types we have left are:
>
> (A) QAPIExpression,
> (B) JSONValue,
> (C) _ExprValue.
>
> The argument I'm making here is that QAPIExpression and JSONValue are
> distinct enough to warrant having both types (for now, at least); and
> that _ExprValue is specialized enough to also warrant its inclusion.
>
> (Brutal honesty: my attempts at unifying this even further had even more
> hacks and unsatisfying conclusions, and fully unifying these types
> should probably wait until we're allowed to rely on some fairly modern
> Python versions.)

Feels okay to me.