John Snow <js...@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.