sarugaku / shellingham

Tool to Detect Surrounding Shell
ISC License
222 stars 33 forks source link

[Question] Are you open to adding type annotations? #69

Open Secrus opened 1 year ago

Secrus commented 1 year ago

Hi. Are the maintainers open to accepting the addition of type annotations to this project? I am willing to do the work that's needed. If the answer is yes, a few extra questions:

  1. Should those be in-code or as separate .pyi files?
  2. Since the project supports 3.7+, would you accept the from __future__ import annotations import way (newer, nicer typing syntax) or should I go with "classic" type classes from the typing module?
  3. Should I add type-checking as part of the CI/tox task?
uranusjr commented 1 year ago

Since we already support only >=3.7, annotations should be in source. Personally I am still waiting for co_annotations, but if you are able to also support the later conversion, using annotations should be fine when it’s useful. And yes a type-checking job should be added if type hints are added. It’s probably a good chance to switch to pre-commit hooks.

Secrus commented 1 year ago

Don't know how the co_annotations syntax will be different, but sure, I can take care of the later conversion. As to mypy as pre-commit hook, it only runs on one Python version, while the issues can be different between Python versions, so I would rather add it as a CI step (or tox entry since I see this is what you are using here).

uranusjr commented 1 year ago

Mypy has a --python-version flag you can use to switch the Python version it checks against. And running type checks against the lowest supported version is good enough in practice.