Quality audit PR 6/6: tooling and CI modernization#324
Merged
Conversation
- tox.ini [testenv:ruff]: add `ruff format --check` so formatting is a CI gate rather than an in-place reformat. - Delete [testenv:black]; drop black from the tox envlist and the main.yml CI matrix. ruff format is now the single formatter. - Remove [tool.black] from pyproject.toml. ruff format was already clean, so no separate reformatting commit is needed.
- tox.ini [testenv:codespell]: remove the stray `command = codespell` key. toxs real setting is `commands`; the singular typo was silently ignored. The valid `commands = codespell .` block is kept. - Add codespell to the tox envlist and the main.yml CI matrix so spelling is now enforced. Local run is clean; no findings to fix. - tox.ini [testenv:docs]: drop the `whitelist_externals` block (removed in tox 4; the still-present `allowlist_externals` already lists rm/mkdir) along with its unused `cd` entry. mypy is intentionally left out of CI: PR 4 documented 11 non-gating precision errors, so it remains a local-only convenience env (no envlist entry).
- main.yml: restrict the `push` trigger to `branches: [develop]` so
feature-branch pushes no longer double-run alongside their PR (the
`pull_request` trigger still fires for every PR, including this one).
- main.yml: add a top-level concurrency group keyed on workflow + ref
with cancel-in-progress, so a superseded push or PR update cancels the
stale run instead of queueing a duplicate.
- main.yml: add top-level `permissions: {contents: read}`. Both jobs
only check out and run tests/tox and never post PR comments, so no job
needs write scopes.
- stale.yml: grant the stale job `issues: write` + `pull-requests: write`
(the only scopes actions/stale needs to label/close stale items); the
default token is otherwise read-only under hardened defaults.
- main.yml: add a dedicated `windows` job (windows-latest, Python 3.12, `tox -e py312`). It passes the `--no-cov` posarg because the 100% coverage gate is unreachable on Windows: POSIX-only branches never execute there and os_specific/posix is already coverage-omitted. The Linux matrix still enforces full coverage. - Set PYTHONUTF8=1 on the job so the unicode-marker tests survive pytests fd-level capture against the legacy Windows code page. The suite is otherwise Windows-aware (tests self-clear TERM and branch on os.name), so no TERM/terminal env is required. - Delete circle.yml (dead CircleCI 1.0 config) and appveyor.yml in the same commit; the new windows job is AppVeyors replacement.
- github/codeql-action/{init,autobuild,analyze}: @v2 -> @V3 (v2 is
deprecated and stops receiving updates).
- actions/checkout: @V3 -> @v6 to match main.yml.
- Drop `javascript` from the language matrix: the repo tracks zero JS,
so that leg only ever produced an empty analysis. The autobuild `if`
is simplified to the sole remaining python language.
- Replace the defunct GitLab flake8 mirror and the 2019-era
pre-commit-hooks (v2.4.0) with:
- pre-commit/pre-commit-hooks v6.0.0 (end-of-file-fixer,
trailing-whitespace, check-yaml, check-added-large-files)
- astral-sh/ruff-pre-commit v0.15.20 (ruff-check + ruff-format),
matching the pinned ruff and the tox/CI formatter gate.
- Apply the resulting auto-fixes: normalize end-of-file newlines across
several tracked files and trim trailing whitespace in README.rst.
`pre-commit run --all-files` is now green and idempotent.
…late Coverage flags: - pytest.ini: move all --cov* flags out of addopts into the tox [testenv] commands so a bare `pytest -k foo` runs without coverage instrumentation. CI still enforces the 100% gate through tox (verified: the full suite reaches 100.00% and the gate fires via the explicit-cov command; no workflow consumes coverage.xml directly). python_files: - pytest.ini: restrict python_files to tests/*.py (drop progressbar/*.py). Verified --doctest-modules still collects the same 13 progressbar doctests and total collection is unchanged (586 items). - Removing progressbar modules from test-module collection also stops pytests assertion-rewrite temporaries (@py_assert1/@py_assert2) from leaking into progressbar.utils namespace, which had been captured in the API-surface snapshot. Drop those two bogus entries from tests/api_surface_snapshot.json; they were never real public API. Dependencies / build: - pyproject.toml: drop unused tests extras: flake8 (linting is ruff now), pytest-mypy (mypy is local-only), and sphinx (already provided by the docs extra; nothing under tests/ imports it). - pyproject.toml: KEEP setuptools-scm in build-system.requires. The audit flagged it as unused, but an sdist-parity check proved it provides the git file-finder that ships docs/, tests/ data, tox.ini and .github/; dropping it shrinks the sdist 152 -> 90 files and omits the test harness. Added a comment so it is not removed again. MANIFEST.in: - Drop includes for nonexistent files (AUTHORS.rst, requirements.txt, Makefile) and the malformed `recursive-exclude *.html`. The sdist file list is identical before/after this edit. Issue template: - git mv ISSUE_TEMPLATE -> .github/ISSUE_TEMPLATE.md (canonical GitHub location for the default issue template).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
There was a problem hiding this comment.
Pull request overview
This PR finalizes the quality-audit series by modernizing CI/tooling so automated checks actually gate what they claim to, while also making Windows a first-class CI target and removing obsolete CI providers/config.
Changes:
- Modernize tox/CI: retire Black, make
ruff format --checkgating, repair and addcodespellto both tox envlist and the GitHub Actions matrix. - Improve developer ergonomics: move coverage flags out of
pytest.iniand into tox/CI invocation so localpytest -k ...runs stay fast. - CI cleanup/modernization: add a Windows tox job, update workflow action versions, add concurrency cancellation and least-privilege permissions, and delete legacy AppVeyor/CircleCI configs.
Reviewed changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Removes Black env, adds codespell env, and makes ruff formatting a failing check (--check). |
| pytest.ini | Removes global coverage addopts and documents moving coverage control into tox. |
| .github/workflows/main.yml | Adds concurrency + permissions hardening, narrows push trigger, adds codespell to matrix, and introduces a Windows tox job. |
| .github/workflows/codeql.yml | Updates action versions and removes the unused JavaScript language from the CodeQL matrix. |
| .github/workflows/stale.yml | Adds explicit job permissions for the stale bot. |
| .pre-commit-config.yaml | Updates hook repos/versions and replaces flake8 with ruff hooks. |
| pyproject.toml | Removes Black config and trims test extras; documents why setuptools-scm must remain in build requirements. |
| MANIFEST.in | Removes references to non-existent/unwanted files from the sdist manifest. |
| tests/api_surface_snapshot.json | Removes pytest assertion-rewrite pollution entries from the API snapshot. |
| ruff.toml | Minor cleanup (trailing whitespace). |
| README.rst | Fixes trailing whitespace in badge alt text. |
| LICENSE | Removes trailing whitespace at EOF. |
| docs/usage.rst | Removes trailing whitespace at EOF. |
| docs/index.rst | Removes trailing whitespace at EOF. |
| docs/examples.rst | Removes trailing whitespace at EOF. |
| .github/ISSUE_TEMPLATE.md | Removes trailing whitespace at EOF. |
| circle.yml | Deletes obsolete CircleCI 1.0 configuration. |
| appveyor.yml | Deletes obsolete AppVeyor configuration (replaced by GitHub Actions Windows job). |
| benchmarks/results.json | Normalizes JSON formatting at EOF. |
| AGENTS.md | Minor formatting normalization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Final PR of the six-part quality audit (#319, #320, #321, #322, #323).
CI now enforces what it claims to
ruff format --checkgates formatting (both formatters previously ran in fix-mode and could never fail); black is retired — one formatter, one config.command =key) and added to the CI matrix. mypy deliberately stays non-gating (11 documented precision divergences; see PR 4).pushruns only ondevelop(no more double runs per PR), concurrency cancellation for superseded runs, least-privilegepermissionsblocks on every workflow.javascriptlanguage dropped from the matrix.Windows is a first-class CI target
A
windows-latestpy312 job replaces AppVeyor (deleted, along with the dead CircleCI 1.0 config). The dedicated Windows/os-specific tests finally run on every PR.Developer experience
--covmoved out of pytestaddopts: barepytest -k foois fast; tox/CI pass the coverage flags explicitly and the 100% gate still fires (verified).python_filesno longer collects package modules as test modules (doctests still collected — parity verified 13/13). Two pytest assertion-rewrite pollution entries this exposed were removed from the API snapshot.testsextra dropsflake8/pytest-mypy/duplicatesphinx; MANIFEST.in loses references to files that don't exist; the issue template moves to.github/where GitHub actually serves it. Deviation from the audit:setuptools-scmstays inbuild-system.requires— the sdist-parity gate proved it is the git file-finder (dropping it shrinks the sdist from 152 to 90 files, losing the test harness); the original audit finding was wrong.Verification
Full suite at 100.00% coverage via the new explicit-flag path on 3.10/3.12/3.14; ruff + ruff-format-check + pyright + codespell clean;
tox -e ruff,codespellgreen; sdist file-list parity exact (± the intended template/CI-config moves). The workflow changes get their real test on this PR's own CI run — expect one verification round, particularly the first-ever Windows job.