[FEATURE] Branching strategy Phase 2.B - release trigger and versioning docs#557
Conversation
Detects an umbrella overture-schema major/minor bump on main and creates a draft GitHub Release at v<major>.<minor>.0. Publishing the draft is the only trigger for the public PyPI publish (Phase 3). Decisions locked: patch builds stay CodeArtifact-only; release notes are authored manually at release time; only the umbrella package bump cuts a release. Closes #533 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
GitHub Actions
Inactive
…ases Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
GitHub Actions
Inactive
…ft release tag Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
GitHub Actions
Inactive
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
GitHub Actions
Inactive
|
👀 Trying to come back up to speed on this. John McCall (@lowlydba) Unblocking this is my main priority for this week. It'll take me some time to come back up to speed and also do a bit of progressive work aimed setting this process into the overall versioning strategy, but I'm very much on it this week. |
|
This is my current understanding of the state. (Same diagram I just posted on the parent tracking issue #509, but with changes marking the deviations from plan that are specific to this PR. (Note: Diagram is LLM-generated.) 
Let me know if anything should be corrected. |
|
Thanks for this John McCall (@lowlydba). The progress so far is awesome, and only really being held back by slow reviewers. Posting my conceptual level thoughts answering items listed in the description below. I'll do one comment per point. Once we're aligned on conceptual should be quick to do the concrete review. |
I'd rather have a version number change in |
Our intention for the package architecture is to version each package independently. I suspect this change relates to Alex's issue #558; and possibly also to the issue of how to maintain separate GitHub releases and tags for different packages within a monorepo. IMO neither of these issues should be blockers for independently versioning the different packages. Alex's issue #558I shouldn't think this would be a huge problem for Alex Iannicelli (@atiannicelli) and the data pipeline team because they should only need to depend on Maintaining separate releases and tagsUp to now, we've only had a single "sequence" of GitHub releases and release tags that version the schema as a whole.
At first glance, it may seem like versioning multiple packages in a monorepo doesn't work well within the GitHub release paradigm, but in practice it's very doable: we just need one tag-and-release "series" for every package. There are different ways we could do this, but here's a scheme I think would work well:
|
Agreed that GitHub auto-generated notes have terrible quality and won't work—and they moreover probably wouldn't work for per-package releases. I very much like the idea of every PR coming with an antsibull-style changelog fragment for each PR, enforced with a GH action, enabling the final release notes to be fully auto-generated. The process would be essentially that every PR that touches one of the Python package trees would require a changelog fragment under that tree, e.g.: This would allow the releases to be fully auto-generated from the version bumps and should also be helpful in addressing the somewhat frequent complaint that the content of an upcoming schema release is opaque/difficult to dosciver. John McCall (@lowlydba) You mentioned this being too much friction. Can you elaborate? |
This ends up being added friction - to create a release we now need the authoritative version bump PRs and then still have to manually go retrieve the changes and write them up as notes. Per-package versioning can also produce many release drafts so there's added work here (one release draft/package) and potential to miss something. If we want a simpler manual release gate "just to make sure" we could have the GH action a "review issue" mid-run and require a designated approver to indicate "approve" or some such on it before the publish actions are taken? |
…changelog - release-trigger: detect per-package major.minor bumps on main and cut a published GitHub Release per package (tag `<package>-v<major>.<minor>.0`, notes extracted from that package's CHANGELOG.md; umbrella marked Latest) - require-changelog-fragment: new PR check enforcing a changelog update on any major.minor bump (accepts a towncrier fragment or a built CHANGELOG.md) - towncrier: centralize [tool.towncrier] in root pyproject.toml; per-package changelog.d/ with a DRY README that defers to docs/versioning.md (no .gitkeep) - docs/versioning.md: Diataxis rewrite (reference/how-to/why) with a TOC - CONTRIBUTING.md: lean guiding-light rewrite (217 -> 91 lines); richer, workflow-accurate gitGraphs - publish-python-packages: add contents: read for reusable-workflow checkout Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
Wrap each release path (main->patch, main->minor, vnext->major) in a collapsible <details> section with its own workflow-accurate gitGraph. Drop the branch-protection bullet and reduce the CI notes to authoritative links to the vnext-compat and pr-advisory workflows, which comment their own fix steps inline. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
… a PR Add how/when each release path reaches consumers (patch rides the next release to PyPI via the umbrella; minor and major publish to PyPI on merge). Reword the Opening a PR CI notes to point at the checks without narrating. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
Move the inline detect heredoc and the extract_notes heredoc out of release-trigger.yaml into .github/workflows/scripts/detect_version_bumps.py and extract_release_notes.py, matching the existing package-versions.py convention. The workflow steps now just invoke them, so the Python is lint-covered, testable, and free of heredoc quoting. Behavior is unchanged. Also vary the CONTRIBUTING release diagrams across distinct packages (places/base/transportation themes) and note that a no-bump merge still publishes a patch to internal CodeArtifact. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
… any package change Extract the release-creation logic into a reusable composite action, .github/actions/create-package-release, with a typed input contract and a dry-run mode. release-trigger now runs as two jobs: detect emits the bumped packages as a JSON step output, and a fail-fast:false matrix job calls the action once per package, so one bad CHANGELOG cannot block sibling releases. extract_release_notes.py moves into the action as an internal detail, and detect_version_bumps.py emits JSON (and uses pathlib for portability). Broaden the changelog policy: require-changelog-fragment now flags any package with changed files (excluding its own changelog.d/ and CHANGELOG.md) that lacks a fragment, not just major/minor bumps. Update docs/versioning.md and CONTRIBUTING.md to match, and clarify that a no-bump merge still publishes a patch to internal CodeArtifact that is consumable immediately. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
👍🏻 Fully aligned, and that is what is now implemented. The release trigger is a Release management stays entirely inside code versioning + PR review, with no out-of-band "publish" action. |
Adopted as described.
The umbrella #558 (pipeline consumers depending only on the umbrella) is tracked separately and is not a blocker here. |
Agreed on the pattern, and it is in. To your friction question: the friction I flagged earlier was more or less a gut check from my past experiences. Personally, although sometimes it annoys me, I think this approach produces really high quality release notes - Lets Go! Two notes on how it landed:
Config is centralized in the root |
Agreed the draft-release gate was friction, so we dropped it. The release is published directly by the workflow with auto-assembled notes, so there is no manual note-writing and no per-package draft to babysit. For the human checkpoint you are after, we can use a GitHub Environment approval (native required-reviewers gate) on the Phase 3 PyPI publish job rather than a mid-run review issue. Same designated-approver semantics, less machinery, and it sits at the real public-publish boundary instead of on every internal release. Tracked in #509 (Phase 3 - non-blocking for this PR) |
…strategy---phase-2b---release-versions # Conflicts: # .github/workflows/publish-python-packages.yaml Signed-off-by: John McCall <john@overturemaps.org>
GitHub Actions
Inactive
…with any-change policy The new overture-schema-pyspark package (merged from main) lacked a changelog.d/ keeper. Add one so the fragment check and towncrier treat it like the other packages. Also correct the stale 'per user-facing change' line in all keepers to match the any-change policy in docs/versioning.md. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: John McCall <john@overturemaps.org>
GitHub Actions
Inactive
Seth Fitzsimmons (sethfitz)
left a comment
Seth Fitzsimmons (sethfitz)
left a comment
There was a problem hiding this comment.
I mostly have questions that might clarify the docs, but I do have a concern about patch version handling, since not publishing those to PyPI would prevent us from publishing bug fixes without promoting them to minor releases.
| | Component | Owner | Set by | | ||
| |-----------|-------|--------| | ||
| | `<major>.<minor>` | Human | Edited in `pyproject.toml` via a reviewed PR. | | ||
| | `<patch>` | CI | [`compute-version`](../.github/actions/compute-version/action.yml) at publish time. The `pyproject.toml` patch is only a floor. | |
There was a problem hiding this comment.
The
pyproject.tomlpatch is only a floor.
Does that mean pyproject.toml can fall out of date if compute-version builds a newer version than is declared (and publishes it)?
| | Event | Version | Destination | | ||
| |-------|---------|-------------| | ||
| | Push to `vnext` | `<last-published>+dev.<run#>` | CodeArtifact (dev) | | ||
| | Push to `main`, no bump | `<major>.<minor>.<next-patch>` | CodeArtifact | |
There was a problem hiding this comment.
So, explicitly: any push to main (by closing PRs) will publish a new package to CodeArtifact? So a storm of merges will produce a flurry of packages?
I would almost expect +<something> treatment of those so we can flow patch releases to PyPI monotonically.
There was a problem hiding this comment.
a storm of merges will produce a flurry of packages?
I think so, but for the internal CodeArtifact haven't we've decided to accept this in order to ensure internal Overture players always have a pre-release build they can work on?
| |-------|---------|-------------| | ||
| | Push to `vnext` | `<last-published>+dev.<run#>` | CodeArtifact (dev) | | ||
| | Push to `main`, no bump | `<major>.<minor>.<next-patch>` | CodeArtifact | | ||
| | `major.minor` bump on `main` | `<major>.<minor>.0` | Public PyPI | |
There was a problem hiding this comment.
Explicitly (for posterity, not just the PR): PyPI will never receive non-.0 patch packages (automatically)?
Patch merges publish to CodeArtifact only; PyPI is reserved for major.minor bumps.
This reads as decided within the PR description, but it seems problematic to me, since it makes it difficult-to-impossible to publish bug fixes without promoting them to minor versions.
My proposal: extend the major/minor convention to patch — a deliberate patch bump publishes to PyPI like any other bump — and send every no-bump merge to CodeArtifact as +main.N (the + from above). It changes "CI owns the patch version" only by redefining it: CI owns the +main.N interim counter, the human owns the released version.
| 1. Bump `<major>.<minor>` in the package's `pyproject.toml` (reset patch to `0`), | ||
| then run `uvx towncrier build --config pyproject.toml --dir packages/<package>` | ||
| from the repo root to fold its fragments into `CHANGELOG.md`. Minor bumps | ||
| target `main`; major bumps go via `vnext` and reach `main` through a release |
There was a problem hiding this comment.
What's a "release merge"? (Is there something this could link to?)
There was a problem hiding this comment.
A merge that causes a release? Not sure it warrants a line item but we can :) It isn't a concept, just a colloquial phrase. Feel free to suggest alternatives.
The tables above explain what causes a merge, so its just "when you merge, and it causes a release..." in short form.
| type), under the affected package: | ||
|
|
||
| ``` | ||
| packages/<package>/changelog.d/<issue-or-pr>.<type>.md |
There was a problem hiding this comment.
How does towncrier determine where in the CHANGELOG to place fragements? Does it look at the commit where this file was introduced?
There was a problem hiding this comment.
I'd check their docs! Not sure off hand.
I'm largely good with this but I echo Seth's concern about I commented similarly on May 24 in the Phase 2.A PR (#534) but likely the comment got missed in the noise. |
On versioning the main branch internal artifactsApparently (according to certain LLMs I've been talking to), the OTOH, apparently formulating it like PEP 440 has the concept of a post-release, which is something like I'll post another comment with thoughts on what to do with the vnext branch. |
On versioning the vnext branch internal artifactsIn my mind the fundamental question with having both main and vnext is that we likely want two different versioning streams, because:
So you probably want two different streams of version numbers. IMO one of the two streams can be done with the post-release So what do we use for the vnext stream? If we have already used up
This is all a bit complicated, but unless I'm mistaken I think it's what needs to be done. If I'm right and this complexity is needed anyway, then I propose the following versioning scheme:
Then:
|
Co-authored-by: Seth Fitzsimmons <sethfitz@amazon.com> Signed-off-by: John McCall <john@lowlydba.com>
GitHub Actions
Inactive
🗺️ Schema reference docs preview is live!
Note ♻️ This preview updates automatically with each push to this PR. |
Co-authored-by: Seth Fitzsimmons <sethfitz@amazon.com> Signed-off-by: John McCall <john@lowlydba.com>
Signed-off-by: John McCall <john@lowlydba.com>
GitHub Actions
Error
GitHub Actions
Inactive
Co-authored-by: Seth Fitzsimmons <sethfitz@amazon.com> Signed-off-by: John McCall <john@lowlydba.com>
Summary
Phase 2.B of the branching strategy (#533): per-package release detection on
main, plus the versioning and contributor docs. Publishing to PyPI is Phase 3 (#509).Full design and rationale: docs/versioning.md and CONTRIBUTING.md.
What this adds
packages/*versions independently and gets its own release series, tagged<package>-v<major>.<minor>.0.overture-schema, this stays invisible downstream in our current usage patterns.main, cuts a published GitHub Release for each package whosemajor.minorbumped.changelog.d/), then before cutting a release run a command that merges all fragments into a changelog, which then feeds release notes.Decisions
Resolves the TBDs from #533/#509 and this PR's review feedback:
major.minorbumps.towncrierfragments over manual notes or auto-generated changelogs.Notes
vnext-compat,rebase-vnext) already in place; unchanged.Closes #533