-
-
Notifications
You must be signed in to change notification settings - Fork 104
🧪 Enforce type annotations with MyPy #381
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces comprehensive type annotations to enforce type safety across the codebase using MyPy. It adds MyPy configuration for multiple Python versions (3.11-3.13) and creates type definitions to ensure proper type checking.
- Adds explicit return type annotations (
-> None
) to functions that were missing them - Introduces TypedDict classes to define structured types for JSON data handling
- Replaces
os.getenv()
withos.environ[]
for required environment variables to improve type safety
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
print-pkg-names.py | Adds return type annotation to debug function |
oidc-exchange.py | Comprehensive type annotations including TypedDict definitions and improved null handling |
attestations.py | Adds null check for OIDC token detection and environment variable access |
_type_stubs/id.pyi | Creates type stub file for the id module |
.pre-commit-config.yaml | Configures MyPy pre-commit hooks for Python 3.11-3.13 |
.mypy.ini | Adds comprehensive MyPy configuration with strict type checking |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
2eb365b
to
a758b76
Compare
@woodruffw could you take a look? Specifically, I'm interested in possible runtime side effects. If there's no problems at runtime, I'll merge right away. |
Sure, I can look later today 🙂 |
@woodruffw thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM -- I did a pass for side effects and don't see any.
No description provided.