doc: modernization design spec (ruff, uv, Gitea CI matrix, 0.2.3 hotfix)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,112 @@
|
|||||||
|
# app_skellington modernization — design
|
||||||
|
|
||||||
|
Date: 2026-10-04 · Status: draft for review
|
||||||
|
|
||||||
|
## Goal
|
||||||
|
|
||||||
|
Keep the library's existing behaviour while modernizing the tooling and backing every compatibility claim with CI. After that, harden the code. The first user-visible result is a **0.2.3 hotfix release**. Every install of the current PyPI release (0.2.2) is broken: `import app_skellington` raises `ImportError ... partially initialized module ... (most likely due to a circular import)`. The fix (`d155176`) is already on `main` but was never released.
|
||||||
|
|
||||||
|
## Decisions (agreed in brainstorming)
|
||||||
|
|
||||||
|
| Topic | Decision |
|
||||||
|
|---|---|
|
||||||
|
| Python floor | Keep `requires-python >=3.8`. When an EOL version starts failing, drop it rather than contort the code (policy recorded in CLAUDE.md). |
|
||||||
|
| Platforms | Claim only what's tested: **Linux**, CPython 3.8–3.14. Windows and macOS are a stated long-term goal for this repo, but out of scope until a runner exists. |
|
||||||
|
| Formatter/linter | **ruff** replaces black, isort and flake8. |
|
||||||
|
| Line length | **88**: the black and ruff default and the de facto modern standard. `main` is already formatted at 88. Measured churn: 4 files, cosmetic only (assert-message wrapping). |
|
||||||
|
| Reformatting | Goes in its **own style-only PR**. |
|
||||||
|
| Build system | Keep pyproject + setuptools + setuptools_scm (tag-driven versions). Add **uv** for envs, the lockfile, builds and publishing. Poetry is already gone from `main`. |
|
||||||
|
| PyPI | Nothing gets published until CI is green across the matrix. Each release goes to TestPyPI first. Publishing stays manual (Mathew's credentials); there's no publish job in CI for now. |
|
||||||
|
| Branches | Mathew recreates `develop` from `main`. All PRs below target `develop`, `develop` goes to `main` by PR, and release tags go on `main`. Stale PR #2 (develop → main, 2024) gets closed as superseded. |
|
||||||
|
|
||||||
|
## Current state (verified 2026-10-04)
|
||||||
|
|
||||||
|
- The suite passes on 3.8, 3.9, 3.10, 3.12, 3.13 and 3.14 from `main`, but it covers only `cfg` plus one CommandTree constructor smoke test. Passing it says little about CLI, DI or logging.
|
||||||
|
- `app_skellington/_version.py` is tracked, yet setuptools_scm rewrites it on every install or build. Newer setuptools_scm writes `tuple[int | str, ...]` annotations, which crash when imported on Python 3.8 and 3.9.
|
||||||
|
- Lint isn't clean: black would reformat 2 files, isort flags 3, flake8 reports W391 and W503.
|
||||||
|
- The README names the wrong debug env var (`APPSKELLINGTON_ENABLE_LOGGING`; the code reads `APPSKELLINGTON_DEBUG`). It also claims Windows and Mac support that nothing tests.
|
||||||
|
|
||||||
|
## Delivery: five PRs in order, then the release
|
||||||
|
|
||||||
|
### PR 1: style (ruff, format only)
|
||||||
|
|
||||||
|
- `pyproject.toml`: `[tool.ruff]` with `line-length = 88` and `target-version = "py38"`, excluding `app_skellington/_version.py`. Lint rules are `select = ["E", "F", "W", "I"]`, matching the old flake8 + isort coverage.
|
||||||
|
- Any existing violation that `ruff check --fix` can't fix safely gets a per-rule `ignore` with a comment. Each one is removed during hardening (PR 5). This PR changes no logic.
|
||||||
|
- Apply `ruff format` and the safe `ruff check --fix` (import order).
|
||||||
|
- Delete `.flake8`. In `.pre-commit-config.yaml`, replace the black, isort and flake8 hooks with `ruff-pre-commit` (`ruff-check --fix`, `ruff-format`). That also drops the `python3.8` pin, which breaks on machines without 3.8.
|
||||||
|
- Done when `ruff check` and `ruff format --check` are clean, tests pass, and the diff is whitespace and import order only.
|
||||||
|
|
||||||
|
### PR 2: tooling (uv), packaging hygiene, docs
|
||||||
|
|
||||||
|
- `[dependency-groups] dev = ["ruff", "pytest", "pre-commit", "pyyaml"]` (PEP 735) replaces `[project.optional-dependencies] dev`, so dev tools are no longer extras on the published package. Commit `uv.lock`. uv resolves one lock across all Pythons in range, so 3.8 lanes get compatible older pytest and other tools with no hand pinning.
|
||||||
|
- `.python-version`: set the dev default to one current version (3.14). CI covers the rest.
|
||||||
|
- **Untrack `_version.py`** and add it to `.gitignore`. setuptools_scm still generates it at build time, so the sdist and wheel ship it. Nothing in the package imports it.
|
||||||
|
- Commands become `uv sync`, `uv run pytest`, `uv run ruff ...`, `uv build` and `uv publish`.
|
||||||
|
- README:
|
||||||
|
- fix the debug env var
|
||||||
|
- replace the pyenv and pip dev setup with uv
|
||||||
|
- drop the untested Windows/Mac claim; PR 3 adds the tested-platform line once CI backs it
|
||||||
|
- update the test and publish instructions
|
||||||
|
- leave the claim "config.ini is created if missing" until PR 4 confirms it; correct it there if the test disagrees
|
||||||
|
- Update CLAUDE.md to match: commands, the branch section after `develop` is recreated, and the lint and `_version.py` notes.
|
||||||
|
|
||||||
|
### PR 3: Gitea CI (checks only)
|
||||||
|
|
||||||
|
`.gitea/workflows/ci.yaml`, following `~/.claude/docs/gitea-ci-pattern.md`:
|
||||||
|
|
||||||
|
- **Triggers:** `pull_request` (types `opened, synchronize, reopened, edited`), `push: [develop, main]`, `workflow_dispatch`. Concurrency cancels superseded PR runs only.
|
||||||
|
- **WIP gate** on every job that runs on `pull_request`: skip when the title starts with `WIP:` or `[WIP]`, and nothing else.
|
||||||
|
- **`lint`:** one Python. Runs `uv run ruff check` and `uv run ruff format --check`.
|
||||||
|
- **`test`:** matrix over `python: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14"]`, `fail-fast: false`. Container `python:<v>`. The first step installs `nodejs` (Gitea actions are node programs), then uv. Checkout uses `fetch-depth: 0`, because setuptools_scm needs tags. Then each lane:
|
||||||
|
1. `uv sync --locked`, then `uv run pytest`.
|
||||||
|
2. **Packaging smoke test:** `uv build`, install the built wheel into a fresh venv, then `cd /` (away from the source tree) and run `python -c "import app_skellington; ..."`. This is the check that would have caught 0.2.2.
|
||||||
|
- **`ci-ok`** aggregate job, `needs: [lint, test]`, with `if: always()` plus the WIP gate. It fails unless every needed job's result is `success`.
|
||||||
|
- Why it runs `always()`: Gitea reports a *skipped* job as a `success` status. A plain `needs:` aggregate would go green when a needed job failed. The `always()` + explicit check closes that hole.
|
||||||
|
- Branch protection then requires only `CI / ci-ok (pull_request)`, so one required check covers every matrix lane.
|
||||||
|
- **`tests/test_ci_workflow.py`** pins the invariants: triggers, WIP gate on each PR job, matrix versions equal to the `requires-python` floor through the newest, and the `ci-ok` logic. CI drift then shows up as a red test.
|
||||||
|
- README: add "Tested on Linux, CPython 3.8–3.14 (CI). Windows/macOS: goal, untested."
|
||||||
|
- **Rollout per the playbook:** run every gate locally first, then canary one gate at a time (a lint failure, then a test failure), watch it go red, revert, watch it go green. Mathew then adds the branch-protection status check.
|
||||||
|
|
||||||
|
### Release 0.2.3 (hotfix, after PR 3 is green on `main`)
|
||||||
|
|
||||||
|
1. Promote `develop` → `main` by PR, with CI green.
|
||||||
|
2. Tag `v0.2.3` on `main`, then `uv build`.
|
||||||
|
3. `uv publish` to **TestPyPI**. Install it from TestPyPI on every matrix Python (a script under `scripts/` running `uv run --isolated --python X --index testpypi`) and import it.
|
||||||
|
4. `uv publish` to PyPI. Mathew does this step; whether to yank 0.2.2 afterwards is his call.
|
||||||
|
5. CHANGELOG entry.
|
||||||
|
|
||||||
|
### PR 4: characterization tests
|
||||||
|
|
||||||
|
These pin *current* behaviour before any hardening changes it. Known bugs get tests marked `xfail(strict=True)`, each with the bug named, so a fix flips the test and forces it to be updated.
|
||||||
|
|
||||||
|
- **CLI:** single-command and submenu dispatch, nested submenus, defaults versus required positionals, help text from the first docstring sentence, `NoCommandSpecified`, option placement (root versus command).
|
||||||
|
- **Container and DI:** factory registration, `ServiceNotFound`, constructor-parameter-name injection, `register_class_as_commands` (fresh instance per call, public methods only), the hook order `_cli_options` → `_services` → `_command_menu`.
|
||||||
|
- **Logging:** `transform_config` (`root` → `""`, level strings to ints, `version=1`, relative file path → user log dir, using a tmp dir), `APPSKELLINGTON_DEBUG` on and off.
|
||||||
|
- **Config:** whether config.ini is created when missing (then correct the README to match), `$var` interpolation, spec defaults, validation failure returning `False`.
|
||||||
|
- **Public API:** everything `__init__.py` exports can be imported.
|
||||||
|
|
||||||
|
### PR 5+: hardening, one fix per PR or small groups
|
||||||
|
|
||||||
|
Each fix flips its `xfail` from PR 4. Known so far:
|
||||||
|
- `Config.__delitem__` recurses forever (it calls `del self[key]`).
|
||||||
|
- `_util.get_asset` has a broken `isinstance(module, module)` check.
|
||||||
|
- `cli._lookup_command` can stop at `input("<broken>")`.
|
||||||
|
- `logger.warn` is deprecated; use `warning`.
|
||||||
|
- `_load_config_files` swallows broad exceptions.
|
||||||
|
- `Config.__contains__` returns `None` on error.
|
||||||
|
- Remove the per-rule ruff ignores added in PR 1.
|
||||||
|
- Issue #3: appdirs → platformdirs, with a path-parity check.
|
||||||
|
|
||||||
|
The public API (the names exported from `__init__.py`) stays stable.
|
||||||
|
|
||||||
|
## Out of scope (recorded, not planned)
|
||||||
|
|
||||||
|
- Windows and macOS CI runners. This is the long-term multi-platform goal and needs infrastructure.
|
||||||
|
- A PyPI publish job in CI (would need the deferred credential model in the CI pattern doc).
|
||||||
|
- A PyPy lane and type checking. Both can be revisited once the matrix is stable.
|
||||||
|
|
||||||
|
## Risks
|
||||||
|
|
||||||
|
- **Old Python images:** `python:3.8` images are frozen, since 3.8 is EOL. If apt or nodejs installs inside them break, the policy applies: drop the lane and raise the floor rather than build workarounds.
|
||||||
|
- **Lockfile resolution on 3.8:** some dev tools may have no 3.8-compatible release. uv's fork resolution should handle this; if it can't, that's the trigger to raise the floor.
|
||||||
|
- **setuptools_scm in CI** needs full history and tags (`fetch-depth: 0`, `safe.directory`). Without them the version falls back to `0.0.0`, and the smoke test should assert that it doesn't.
|
||||||
Reference in New Issue
Block a user