Fix mypy configuration and tighten typing coverage - #2
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes mypy configuration and tightens typing coverage to ensure strict type checking across the codebase. It addresses configuration issues and adds missing type annotations for better type safety.
- Fixed mypy exclude regex configuration to be properly formatted
- Added explicit return type annotations to test functions and fixtures
- Improved type safety in utility functions with better type annotations
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mypy.ini | Simplified exclude regex pattern for third-party directories |
| tests/test_slugify.py | Added return type annotation to test function |
| tests/test_new_list_toc.py | Added return type annotation to test function |
| tests/test_date_envs.py | Added pytest import and type annotations for test functions |
| tests/test_cli_init.py | Added return type annotation to test function |
| tests/conftest.py | Added Iterator import and return type annotation for fixture |
| src/decree/utils.py | Improved type annotations and variable handling in resolve_date |
| src/decree/core.py | Added NoReturn type and improved date handling consistency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| actual_env: Mapping[str, str] = os.environ if env is None else env | ||
| if d := actual_env.get("ADR_DATE"): | ||
| return d | ||
| tz = env.get("DECREE_TZ", "UTC") | ||
| tz = actual_env.get("DECREE_TZ", "UTC") |
There was a problem hiding this comment.
[nitpick] The variable name actual_env is verbose and doesn't add clarity. Consider using a shorter name like env_dict or simply reassigning to env.
| slug = slugify(title) | ||
| path = self.dir / f"{number:04d}-{slug}.md" | ||
| tpl = template.read_text(encoding="utf-8") if template else DEFAULT_TEMPLATE | ||
| record_date = date or resolve_date() |
There was a problem hiding this comment.
The variable record_date is computed but then used inconsistently. The AdrRecord constructor uses record_date while the template formatting also uses record_date, but the logic could be clearer by computing this value once at the beginning of the method.
| title=title, | ||
| status=status.value, | ||
| date=(date or resolve_date()), | ||
| date=record_date, |
There was a problem hiding this comment.
The variable record_date is computed but then used inconsistently. The AdrRecord constructor uses record_date while the template formatting also uses record_date, but the logic could be clearer by computing this value once at the beginning of the method.
| path.write_text(content, encoding="utf-8", newline="\n") | ||
| return AdrRecord( | ||
| number=number, slug=slug, title=title, status=status, date=resolve_date(), path=path | ||
| number=number, slug=slug, title=title, status=status, date=record_date, path=path |
There was a problem hiding this comment.
The variable record_date is computed but then used inconsistently. The AdrRecord constructor uses record_date while the template formatting also uses record_date, but the logic could be clearer by computing this value once at the beginning of the method.
|
To use Codex here, create an environment for this repo. |
f5de698
into
codex/bootstrap-decree-repo-with-initial-files
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68e4720714108326b7f80d65e09cbc42