Page MenuHomeSoftware Heritage

typing: minimal changes to make a no-op mypy run pass
ClosedPublic

Authored by zack on Sep 24 2019, 11:05 AM.

Diff Detail

Repository
rDDEP Push deposit
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

zack created this revision.Sep 24 2019, 11:05 AM
zack updated this revision to Diff 6828.Sep 25 2019, 11:08 AM
  • models: add required on_delete kwarg to ForeignKey
  • models: add migration to on_delete=protect
  • typing: minimal changes to make a no-op mypy run pass
zack updated this revision to Diff 6829.Sep 25 2019, 11:41 AM
  • typing: convert leftover variable annotation to comments
ardumont added inline comments.
swh/deposit/migrations/0017_auto_20190925_0906.py
21 ↗(On Diff #6829)

This is out of scope for this diff, this is D2031's
You need to arc diff HEAD~3 --update D2032 (3 commits you made for this diff).

As the diff is stacked (i don't see the Depends on D2031 in the diff description but you probably added the diff's dependency yourself), the build should be ok.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 26 2019, 3:49 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
zack reopened this revision.Sep 27 2019, 10:34 AM
zack updated this revision to Diff 6834.Sep 27 2019, 10:35 AM
  • typing: minimal changes to make a no-op mypy run pass
  • typing: convert leftover variable annotation to comments
  • init.py: switch to documented way of extending path
zack added a comment.Sep 27 2019, 10:36 AM

this is now (also) waiting for an upstream release of django-stubs compatible with mypy >= 0.730

zack updated this revision to Diff 6903.Oct 1 2019, 12:28 PM
  • tox: add mypy environment

where should the mypy_django_plugin dependency be? looks like this is required.

zack added a comment.Oct 2 2019, 11:46 AM

where should the mypy_django_plugin dependency be? looks like this is required.

It comes from the django-stubs PyPI package, which is added by this diff to requirements-test.txt. But. The most recent release of django-stubs is incompatible (and declared as such in its dependencies) with the most recent release of mypy (0.730). I'm waiting for a new release of django-stubs to be made by upstream. See discussion here: https://github.com/typeddjango/django-stubs/issues/160

zack planned changes to this revision.Nov 1 2019, 12:26 PM

removing this form the review queue for now, as it will need py3.7 in the CI to pass tests anyway

zack updated this revision to Diff 7601.Nov 1 2019, 3:36 PM
  • typing: minimal changes to make a no-op mypy run pass
  • typing: convert leftover variable annotation to comments
  • init.py: switch to documented way of extending path
  • tox: add mypy environment
  • requirements-test.txt: add back deps from master
  • Makefile.local: sync typecheck target with global Makefile.python
  • mypy: ignore psycopg2
zack planned changes to this revision.Nov 1 2019, 3:36 PM
anlambert added inline comments.
tox.ini
37 ↗(On Diff #7601)

Adding setenv = DJANGO_SETTINGS_MODULE = swh.deposit.settings.testing here will fix the mypy execution with tox

zack updated this revision to Diff 7684.Nov 7 2019, 9:25 AM
  • tox.ini: pass DJANGO_SETTINGS_MODULE to mypy environment
zack updated this revision to Diff 7685.Nov 7 2019, 9:31 AM
  • mypy: properly type get/put methods, as well deposit_requests_types dict