Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tighten mypy: globus_cli.parsing #922

Merged
merged 12 commits into from
Dec 20, 2023
Merged

Tighten mypy: globus_cli.parsing #922

merged 12 commits into from
Dec 20, 2023

Conversation

sirosen
Copy link
Member

@sirosen sirosen commented Dec 20, 2023

Step by step, get the entire subpackage checked strictly under mypy.

- Don't use a subclass of GlobusCommand for env var checking (not necessary)
- Remove the opt-out capability for env var checking (never used)
- Remove the capability for custom class usage (also never used)
- tighten the mypy checking criteria
- for simpler type semantics, remove use of `functools.partial`
- disallow_untyped_defs=false in mypy config
- remove use of functools.partial (never used)
Tighten mypy and remove use of functools.partial (simplify).
rm config for a module which no longer exists.
rm config for a module which no longer exists
@sirosen sirosen added the no-news-is-good-news This change does not require a news file label Dec 20, 2023
src/globus_cli/parsing/commands.py Outdated Show resolved Hide resolved
@kurtmckee
Copy link
Member

Build is failing; I though I saw a deleted from __future__ import annotations, which may be where that's coming from.

@sirosen
Copy link
Member Author

sirosen commented Dec 20, 2023

It turned out to be a different module needing deferred annotations. One of these days, I'll finally learn my lesson and go back to running tox (no args) before opening PRs.

@sirosen sirosen merged commit 9dbcba1 into globus:main Dec 20, 2023
19 checks passed
@sirosen sirosen deleted the tighten-mypy branch December 21, 2023 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-news-is-good-news This change does not require a news file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants