diff --git a/.github/workflows/sync.yml b/.github/workflows/sync.yml index 12febf9..8de857e 100644 --- a/.github/workflows/sync.yml +++ b/.github/workflows/sync.yml @@ -11,15 +11,14 @@ on: - cron: "17 * * * *" # hourly workflow_dispatch: inputs: + # auto: major if oasdiff or the API surface check finds a break, + # otherwise minor. major forces one for a break neither gate sees. + # There is no minor override: a detected break never ships as a minor. bump: description: "Which part of the version to bump" type: choice - options: [minor, major] - default: minor - acknowledge_breaking: - description: "Publish even though oasdiff reports a breaking change" - type: boolean - default: false + options: [auto, major] + default: auto dry_run: description: "Regenerate and build, but don't tag, release or publish" type: boolean @@ -27,7 +26,7 @@ on: permissions: contents: write # commit + tag + create release - issues: write # open an issue when a breaking change is detected + issues: write # open an issue when a release is stuck id-token: write # PyPI trusted publishing (job-scoped, hence one job) concurrency: @@ -35,7 +34,7 @@ concurrency: cancel-in-progress: false env: - # Pinned deliberately: oasdiff decides whether we publish at all. + # Pinned deliberately: oasdiff decides whether a release is a major. OASDIFF_VERSION: "1.32.1" jobs: @@ -112,74 +111,75 @@ jobs: # 1 means it found breaking changes. Anything else means it couldn't # do the comparison at all — a truncated fetch, an HTML error page — - # which is not a breaking change and mustn't be filed as one. + # which is not a breaking change and mustn't be released as one. if [ "$rc" -ne 0 ] && [ "$rc" -ne 1 ]; then echo "::error::oasdiff exited $rc without comparing the schemas." exit 1 fi + # A break does not stop the run: it makes this release a major. + # The report goes into the release notes so the major explains + # itself. if [ "$rc" -ne 0 ]; then echo "breaking=true" >> "$GITHUB_OUTPUT" - { echo "report<> "$GITHUB_OUTPUT" + { + echo "### oasdiff: breaking changes to the API" + echo + echo '```' + echo "$report" + echo '```' + echo + } >> /tmp/breaks.md else echo "breaking=false" >> "$GITHUB_OUTPUT" fi - # A human dispatch with acknowledge_breaking skips this, and that is the - # only way to ship a major — nothing can publish outside this workflow. - - name: Halt on breaking change - if: steps.changed.outputs.changed == 'true' && steps.breaking.outputs.breaking == 'true' && !inputs.acknowledge_breaking - env: - GH_TOKEN: ${{ github.token }} - REPORT: ${{ steps.breaking.outputs.report }} + # Before the version is chosen, because a removal here makes this + # release a major. + - name: Regenerate and check the API surface + if: steps.changed.outputs.changed == 'true' + id: surface run: | set -euo pipefail - body="$(cat <oasdiff report - - \`\`\` - $REPORT - \`\`\` - - - - Triggered by run: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID - EOF - )" - # We stop without committing the new schema, so every later run sees - # the same diff and lands here again. Only file one issue. - existing="$(gh issue list --label breaking-change --state open --limit 1 --json number --jq '.[0].number // empty' 2>/dev/null || true)" - if [ -n "$existing" ]; then - echo "::error::Breaking change detected; issue #$existing is already open." - exit 1 + mv openapi.json.new openapi.json + make generate + make install + # Exit 3 is removals only; anything else is a broken build and + # stops the run. + .venv-verify/bin/python scripts/verify_import.py api-surface.txt 2>/tmp/surface.txt && rc=0 || rc=$? + cat /tmp/surface.txt >&2 + if [ "$rc" -eq 3 ]; then + echo "removed=true" >> "$GITHUB_OUTPUT" + { + echo "### Removed from the Python API" + echo + echo '```' + grep '^ ' /tmp/surface.txt + echo '```' + echo + } >> /tmp/breaks.md + elif [ "$rc" -ne 0 ]; then + exit "$rc" + else + echo "removed=false" >> "$GITHUB_OUTPUT" fi - # Created here rather than assumed: gh issue create fails outright on - # a label that doesn't exist, and an unlabelled fallback would be - # invisible to the dedup above, filing one issue an hour forever. - gh label create breaking-change \ - --description "Schema changed incompatibly; release halted" \ - --color B60205 2>/dev/null || true - - gh issue create \ - --title "Breaking API change detected — manual release required" \ - --body "$body" \ - --label "breaking-change" - echo "::error::Breaking change detected; opened an issue and stopped." - exit 1 - - # Scheduled runs only ever bump minor, since anything breaking has already - # stopped the run. Patch is for hand-fixes, major for a human dispatch. + # After the surface check, because it is one of the two signals. Patch + # is for hand-fixes and never cut here. - name: Decide next version if: steps.changed.outputs.changed == 'true' id: version + env: + BUMP_INPUT: ${{ inputs.bump || 'auto' }} + BREAKING: ${{ steps.breaking.outputs.breaking }} + REMOVED: ${{ steps.surface.outputs.removed }} run: | set -euo pipefail - BUMP="${{ inputs.bump || 'minor' }}" + if [ "$BUMP_INPUT" = "major" ] || [ "$BREAKING" = "true" ] || [ "$REMOVED" = "true" ]; then + BUMP=major + else + BUMP=minor + fi # Highest version, not `git describe`'s nearest-by-commit-distance: # a hand-cut patch tag would otherwise make us recompute a version # that already exists. The pattern also skips any non-semver tag, @@ -200,10 +200,25 @@ jobs: exit 1 fi - echo "Next version: $NEXT (base: ${BASE:-none}, bump: $BUMP)" + echo "Next version: $NEXT (base: ${BASE:-none}, bump: $BUMP, oasdiff break: $BREAKING, surface removals: $REMOVED)" echo "next=$NEXT" >> "$GITHUB_OUTPUT" - # Before make verify, so the wheel we check is the wheel we publish. + # Release notes: the breaks, if any, above GitHub's generated notes. + if [ "$BUMP" = "major" ]; then + { + echo "## Breaking changes" + echo + if [ -f /tmp/breaks.md ]; then + cat /tmp/breaks.md + else + echo "Released as a major by hand (bump: major); neither gate reported a break." + fi + } > /tmp/release-notes.md + else + : > /tmp/release-notes.md + fi + + # Before make test, so the wheel we check is the wheel we publish. - name: Set the release version if: steps.changed.outputs.changed == 'true' env: @@ -213,12 +228,14 @@ jobs: sed -i -E "0,/^version = /s/^version = .*/version = \"${NEXT#v}\"/" pyproject.toml grep -m1 '^version = ' pyproject.toml - - name: Regenerate and check + - name: Record the surface and test if: steps.changed.outputs.changed == 'true' run: | set -euo pipefail - mv openapi.json.new openapi.json - make generate + # Rewritten on every release, removals and additions included, so + # the next release checks against what this one ships. Before the + # tests, so the check inside make verify runs against it. + make surface # make test depends on make verify: builds the wheel, imports every # module, type-checks, checks the API surface, then runs the tests. # The generated code changes on every release, which is exactly when @@ -229,7 +246,7 @@ jobs: # next run sees the same schema diff and fails identically. Left alone # that is a red cron every hour and no other signal. - name: Report a stuck release - if: failure() && steps.changed.outputs.changed == 'true' && steps.breaking.outputs.breaking != 'true' + if: failure() && steps.changed.outputs.changed == 'true' env: GH_TOKEN: ${{ github.token }} run: | @@ -250,7 +267,7 @@ jobs: Run: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID - Likely causes: the generator rejected the new schema, a post-generation fix in \`scripts/\` stopped matching, or verification failed. + Likely causes: the generator rejected the new schema, a post-generation fix in \`scripts/\` stopped matching, or verification failed. Removals from the API surface do not land here: they make the release a major. EOF )" @@ -264,6 +281,8 @@ jobs: echo echo "Would have released **$NEXT**." echo + cat /tmp/release-notes.md + echo echo "Built artefacts:" echo '```' ls -1 dist @@ -288,7 +307,9 @@ jobs: # refused, and every later run recomputes this version and dies on # "tag already exists". git push --atomic origin HEAD "refs/tags/$NEXT" - gh release create "$NEXT" --generate-notes --title "$NEXT" + # --notes-file with --generate-notes puts the file above the + # generated notes. The file is empty on a minor. + gh release create "$NEXT" --generate-notes --notes-file /tmp/release-notes.md --title "$NEXT" # Has to live in this job: id-token is job-scoped, and a tag pushed with # GITHUB_TOKEN won't trigger a separate workflow. diff --git a/Makefile b/Makefile index 4e74a59..4c70450 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ RUFF_VERSION := 0.16.8 PACKAGE := incident_io SCHEMA_URL := https://api.incident.io/v1/openapiV3.json -.PHONY: fetch generate verify test surface clean +.PHONY: fetch generate install verify test surface clean # -L because without it a redirect is a silent success writing zero bytes, # which parses as an empty schema. OUT lets the release workflow fetch to a @@ -49,13 +49,18 @@ generate: openapi.json # halt the hourly pipeline rather than fail a pull request. MYPY_VERSION := 2.3.1 -# Python has no compile step, so we install the wheel and exercise it. See -# scripts/verify_import.py for why importing the package alone isn't enough. -verify: +# Build the wheel and install it into .venv-verify, with what verify and test +# run. Separate from verify so the release can run the surface check itself +# and tell a removal (exit 3) from a broken build. +install: rm -rf dist uv build uv venv --clear .venv-verify uv pip install --quiet --python .venv-verify dist/*.whl mypy==$(MYPY_VERSION) pytest pytest-asyncio + +# Python has no compile step, so we install the wheel and exercise it. See +# scripts/verify_import.py for why importing the package alone isn't enough. +verify: install .venv-verify/bin/python scripts/verify_import.py api-surface.txt # We ship py.typed, so consumers' type checkers read these annotations. Run # against the installed package rather than the source tree: on the tree, @@ -68,9 +73,9 @@ verify: test: verify .venv-verify/bin/python -m pytest tests/ -q -# Accept the current surface as the new baseline. Run after a deliberate -# breaking change, alongside the major version bump. -surface: verify +# Accept the current surface as the new baseline, removals included. The +# release runs this itself on every release. +surface: install .venv-verify/bin/python scripts/api_surface.py write $(PACKAGE) api-surface.txt clean: diff --git a/README.md b/README.md index f4d17a2..7226039 100644 --- a/README.md +++ b/README.md @@ -122,8 +122,8 @@ default, so run with `-W default::DeprecationWarning` to see them. Releases are cut automatically whenever the API schema changes. We use [SemVer](https://semver.org/): additive API changes bump the minor version. -Changes that would break existing code are never released automatically - they -require a deliberate major version. +Changes that would break existing code bump the major version, and the release +notes list what broke. ## Support diff --git a/scripts/api_surface.py b/scripts/api_surface.py index ead8e93..e9ee63d 100644 --- a/scripts/api_surface.py +++ b/scripts/api_surface.py @@ -23,6 +23,10 @@ python scripts/api_surface.py check incident_io api-surface.txt Run against the installed wheel, so it describes what consumers get. + +`check` exits 3 when the only problem is removed entries. Anything else that +goes wrong exits 1. The release reads 3 as "this is a major" and 1 as a broken +build. """ from __future__ import annotations @@ -117,11 +121,11 @@ def check(current: list[str], recorded_path: Path) -> int: print(f" ... and {len(removed) - 40} more", file=sys.stderr) print( "\nConsumers import these by name, so removing one breaks them at import " - "or call time even when the wire contract is unchanged. This needs a major " - "version, not the automatic minor bump.", + "or call time even when the wire contract is unchanged. The release cuts " + "a major for it.", file=sys.stderr, ) - return 1 + return 3 def main(argv: list[str]) -> int: