-
Notifications
You must be signed in to change notification settings - Fork 0
Adopt Skylos dead-code detection #244
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| # ADR-003: Use three-tier Python linting | ||
| # ADR-003: Use layered Python linting | ||
|
|
||
| ## Status | ||
|
|
||
|
|
@@ -14,17 +14,22 @@ package code so internal APIs stay discoverable as modules are refactored. | |
| That documentation requirement needs to be part of the normal lint gate rather | ||
| than an optional local check. It also needs to run after the virtual | ||
| environment has been created and synchronized, because Interrogate is installed | ||
| as a development dependency. | ||
| as a development dependency. Cross-module dead-code detection also needs a | ||
| blocking, deterministic production scan, without treating test-only references | ||
| as application liveness. | ||
|
|
||
| ## Decision | ||
|
|
||
| `make lint` is the canonical Python lint gate and runs three tiers in order: | ||
| `make lint` is the canonical Python lint gate and runs four tiers in order: | ||
|
|
||
| 1. Ruff checks formatting-adjacent style and broad correctness rules. | ||
| 2. Interrogate runs with `--fail-under 100` against `lading` and requires 100% | ||
| docstring coverage. | ||
| 3. Pylint runs through the pinned `pylint-pypy-shim` command and applies the | ||
| selected complementary checks. | ||
| 4. Skylos runs separately through a pinned `uv tool run` environment against | ||
| `lading`, with dead-code analysis only, no uploads or provenance collection, | ||
| and no repository-wide grep verification. | ||
|
Comment on lines
+30
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Describe the implemented Skylos command. Replace Proposed fix-4. Skylos runs separately through a pinned `uv tool run` environment against
+4. Skylos runs separately through the locked `uv run` project environment against🤖 Prompt for AI Agents |
||
|
|
||
| The Makefile keeps lint tooling wired as prerequisites as well as recipe | ||
| commands. `lint` depends on `build` before checking `interrogate`, so | ||
|
|
@@ -35,7 +40,9 @@ the virtual-environment tool. | |
|
|
||
| New package modules, helper functions, and refactors must include docstrings at | ||
| the time they are introduced. Missing documentation fails `make lint` before | ||
| the Pylint tier runs. | ||
| the Pylint tier runs. Genuine dead code must be removed. A verified static | ||
| analysis false positive requires a precise, reasoned Skylos entry point or | ||
| named allow-list exception in `pyproject.toml`. | ||
|
|
||
| Contributors can still use Ruff and targeted tests during inner-loop work, but | ||
| changes are not ready until the full `make lint` target succeeds. | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,7 +60,6 @@ class WorkspaceDependency(msgspec.Struct, frozen=True, kw_only=True): | |
| class WorkspaceCrate(msgspec.Struct, frozen=True, kw_only=True): | ||
| """Represents a single crate discovered in the workspace.""" | ||
|
|
||
| id: str | ||
| name: str | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
After removing the AGENTS.md reference: AGENTS.md:L21-L23 Useful? React with 👍 / 👎. |
||
| version: str | ||
| manifest_path: Path | ||
|
|
@@ -164,7 +163,6 @@ def topologically_sorted_crates(self) -> tuple[WorkspaceCrate, ...]: | |
| -------- | ||
| >>> from pathlib import Path | ||
| >>> crate = WorkspaceCrate( | ||
| ... id="a 0.1.0", | ||
| ... name="a", | ||
| ... version="0.1.0", | ||
| ... manifest_path=Path("a/Cargo.toml"), | ||
|
|
@@ -207,7 +205,6 @@ def crates_by_name(self) -> dict[str, WorkspaceCrate]: | |
| -------- | ||
| >>> from pathlib import Path | ||
| >>> crate = WorkspaceCrate( | ||
| ... id="a 0.1.0", | ||
| ... name="a", | ||
| ... version="0.1.0", | ||
| ... manifest_path=Path("a/Cargo.toml"), | ||
|
|
||
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restrict Markdown linting to tracked files.
Replace the whole-tree
findscan at Line 108 with a tracked-file list fromGit. An untracked Markdown file outside the excluded directories can currently
fail
make markdownlint, which breaks the stated tracked-documentation scope.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents