From ed63a2f70779a63a8057dbfc6b0d0fca534c6e74 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 22 Sep 2026 12:04:10 -0400 Subject: [PATCH] docs(contributing): correct four statements the recent merges falsified The *Coding style* section still told contributors that none of the linters is run by the pull-request workflows, which #626 made false when it added `.github/workflows/lint.yml`. Saying only that they now run would mislead in the other direction, so the replacement leads with the part that decides what a contributor should expect: all four steps carry `continue-on-error: true`, so a red linter cannot fail a pull request. It also keeps `isort` out of that group -- isort appears in `cron-vendor.yml` as a formatter over the regenerated constants, not as a check -- and records the `make vermin` trap, whose exit status is the viewer's rather than vermin's, which is why `make vermin-ci` is the target CI runs. Three smaller corrections found while checking the rest of the file against the tree: * The README has no *Testing* section any more. Its only mention of testing is a row in the *Documentation* table linking to `docs/source/testing.rst`, which is where the test commands moved. * The `Changelog drift` job's triggers are scoped to `main` -- `push` and `pull_request` both carry `branches: [main]` -- rather than firing on every push and pull request in the repository. * "The exception stops at the root" overstated the Markdown boundary: the three issue and pull-request templates under `.github/` are Markdown for the same GitHub-rendering reason the root files are. No changelog entry, deliberately: none of this is user-visible, and a bullet here would collide with the changelog consolidation currently in flight. --- CONTRIBUTING.md | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5cf40386f..5cf911a4f 100755 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,8 +8,8 @@ happens after that. ## Getting started - Fork the repository on GitHub. -- Read the README for installation and build instructions, and its *Testing* section for the test - commands. +- Read the README for installation and build instructions, and the *Testing* page it links from its + *Documentation* table — `docs/source/testing.rst` — for the test commands. - Set up a development environment. `make setup` runs `pipenv install --skip-lock --dev`, and the `Makefile` exports `PIPENV_VENV_IN_PROJECT=1`, so the environment lands in `.venv/` inside the checkout. **Only that environment has the dependencies** — the `make` targets below all run @@ -73,8 +73,8 @@ python util/changelog_md.py --check # exits 0 when they agree, prints a diff ``` The generator needs only the standard library, so it runs against a bare interpreter. The -`Changelog drift` job in `.github/workflows/unit-tests.yml` runs `--check` on every push and pull -request, and a hand-edited `CHANGELOG.md` will fail it. +`Changelog drift` job in `.github/workflows/unit-tests.yml` runs `--check` on every push to `main` +and every pull request targeting it, and a hand-edited `CHANGELOG.md` will fail it. ## Documentation @@ -83,8 +83,9 @@ Documentation is reStructuredText under `docs/source/`, built with `make docs`. The Markdown files at the repository root — this one, `CODE_OF_CONDUCT.md`, `SECURITY.md`, `CHANGELOG.md` and the README — are a deliberate exception to that rule, because their consumers are -GitHub's own rendering and the release body rather than Sphinx. The exception stops at the root: -anything added under `docs/source/` is `.rst`. +GitHub's own rendering and the release body rather than Sphinx. The issue and pull-request templates +under `.github/` are Markdown for the same reason. The exception ends there: anything added under +`docs/source/` is `.rst`. ## Coding style @@ -101,8 +102,27 @@ make bandit # security lint make vermin # minimum-Python-version check ``` -None of these is run by the pull-request workflows, so they are a local gate rather than something a -pull request will fail on. Running them anyway saves a review round. +Four of them — `pylint`, `mypy`, `bandit` and `vermin` — also run in CI, as the `Lint` job in +`.github/workflows/lint.yml`: on every pull request against `main`, on a weekly Saturday schedule +and on demand through `workflow_dispatch`, on Python 3.14 alone rather than across the test matrix. +The job invokes the same `Makefile` targets listed above, with `RUN=` emptying the `pipenv run` +prefix, so what CI checks and what you check locally cannot drift apart. + +**Those steps are advisory, not a gate.** Each carries `continue-on-error: true`, so a finding lands +as a non-blocking annotation and a red linter will not fail your pull request. That is a consequence +of none of the four being clean today; the workflow's header records the current counts and what +each tool would need before its `continue-on-error` line could be deleted. Read the job's run +summary — every step writes its count there — and treat it as information you should not add to. + +`isort` is the exception and is still local-only. It does appear in `cron-vendor.yml`, but as a +formatter that rewrites the regenerated constants rather than as a check, so nothing verifies import +ordering on a pull request. + +One trap in the target list above: `make vermin` writes its report to `temp/vermin.txt` and hands +that file to a viewer — VS Code if it is on `PATH`, otherwise `cat`. The status you get back is +that viewer's rather than vermin's, so a run that found violations still looks like a success. +`make vermin-ci` is the same check without the redirect, and it is the one CI runs. Running the lot +before you push still saves a review round. ### Format of the Commit Message