| Message ID | 20201005195158.2348217-1-jsnow@redhat.com |
|---|---|
| Headers | show |
| Series | qapi: static typing conversion, pt1 | expand |
On Mon, Oct 05, 2020 at 03:51:22PM -0400, John Snow wrote: > Hi, this series adds static type hints to the QAPI module. > This is part one! > > Part 1: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt1 > Everything: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6 > > - Requires Python 3.6+ > - Requires mypy 0.770 or newer (for type analysis only) > - Requires pylint 2.6.0 or newer (for lint checking only) > > In general, this series tackles the cleanup of one individual QAPI > module at a time. Once it passes pylint or mypy checks, those checks are > enabled for that file. > > Type hints are added in patches that add *only* type hints and change no > other behavior. Any necessary changes to behavior to accommodate typing > are split out into their own tiny patches. > > Notes: > > - After patch 07, `isort -c` should pass 100% on this and every > future commit. > FIY, I went on to replicate/validate your testing, and I ran on patches 07 an later: isort --check-only --settings-path ./scripts/qapi/.isort.cfg --recursive ./scripts/qapi With success. > - After patch 08, `flake8 qapi/` should pass 100% on this and every > future commit. > Here on patches 08 an later, I've run: flake8 ./scripts/qapi And starting on patch 24 ("qapi/source.py: add type hint annotations") it complained about: /scripts/qapi/source.py:44:31: F821 undefined name 'T' > - After patch 09, `pylint --rcfile=qapi/pylintrc qapi/` should pass 100% > on this and every future commit. > Here I ran on patches 09 and later: pylint --rcfile=./scripts/qapi/pylintrc ./scripts/qapi/ And all succeeded. > - After patch 18, `mypy --config-file=qapi/mypy.ini qapi/` should pass > 100% on this and every future commit. > Here I ran on patches 18 and later: mypy --config-file=./scripts/qapi/mypy.ini ./scripts/qapi/ And all succeeded. - Cleber.
On 10/5/20 7:05 PM, Cleber Rosa wrote: > Here on patches 08 an later, I've run: > > flake8 ./scripts/qapi > > And starting on patch 24 ("qapi/source.py: add type hint annotations") > it complained about: > > /scripts/qapi/source.py:44:31: F821 undefined name 'T' I think that must be flake8 < 3.8.0. Try using >= 3.8.0. (Yes, still working on getting proper pipenv/requirements/something up for these, sorry.)
On Mon, Oct 05, 2020 at 07:57:18PM -0400, John Snow wrote: > On 10/5/20 7:05 PM, Cleber Rosa wrote: > > Here on patches 08 an later, I've run: > > > > flake8 ./scripts/qapi > > > > And starting on patch 24 ("qapi/source.py: add type hint annotations") > > it complained about: > > > > /scripts/qapi/source.py:44:31: F821 undefined name 'T' > > I think that must be flake8 < 3.8.0. Try using >= 3.8.0. > ACK, I was on 3.7.9. They're all passing now with 3.8.4. - Cleber.
John Snow <jsnow@redhat.com> writes: > Hi, this series adds static type hints to the QAPI module. > This is part one! > > Part 1: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt1 > Everything: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6 > > - Requires Python 3.6+ > - Requires mypy 0.770 or newer (for type analysis only) > - Requires pylint 2.6.0 or newer (for lint checking only) > > In general, this series tackles the cleanup of one individual QAPI > module at a time. Once it passes pylint or mypy checks, those checks are > enabled for that file. > > Type hints are added in patches that add *only* type hints and change no > other behavior. Any necessary changes to behavior to accommodate typing > are split out into their own tiny patches. Neat and pleasant to review. The care you put into this shows. Thanks!