docs: retire CHANGELOG.md in favour of the generated release notes - #88
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c8345c42a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c8345c42a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
@codex ready to merge, per a last review? |
The file arrived as a side effect of a feature PR (fde24c4, the hidden-option work) rather than as a release-process decision, and nothing consumes it: no workflow, no script, no packaging step, and no document links to it. Meanwhile CI already publishes what it was trying to be. The release job runs `gh release create "v${VERSION}" --generate-notes`, so every published version gets notes generated from the merged pull requests and anchored to the version a consumer installs. The file could never be anchored that way — its own header said so, because Nerdbank.GitVersioning assigns the version at pack time — and a review lens correctly flagged that a breaking change pinned to "the commit closing issue It was also the single point of conflict between concurrent pull requests, which is how the question came up: it was the only conflicting file when #85 merged and again when #80 rebased, with three more PRs open behind them. The one piece of guidance that lived only here is migrated: the recipe for restoring the pre-policy exit codes, with its test-suite and MCP consequences, now sits in docs/configuration-reference.md beside the table it talks about. The rest was already covered by the topic pages — docs/commands.md:172 for the additive `isHidden` export fields, docs/testing-toolkit.md for CommandExecution.ExitCode. docs/publishing.md now states where release notes come from and what that asks of a PR description, so the next person does not recreate the file.
…s guidance
Three findings on this PR, all against prose I wrote here. Two are corrections to
claims; the first turned out to be a latent CI defect that the claim exposed.
- `gh release create` ran without `--target`, and gh creates a missing tag from the
latest state of the default branch. The release job also triggers on `release/**`,
so a release-branch publish could tag `main` while attaching packages built from
the branch — which made the new claim that notes are anchored to the installed
version false, and was wrong regardless of the claim. It now passes
`--target "${GITHUB_SHA}"`. An existing tag is unaffected; this only governs the
automatic creation path.
- The guidance said a PR description *is* the release notes. `--generate-notes`
publishes pull request titles, authors and links, not PR bodies, so a migration
step written only in a description is reachable through the link but absent from
the notes. docs/publishing.md now says which is which, and points durable
guidance at the topic pages instead.
- I claimed the rest of the deleted file was already covered by those topic pages.
Two consumer constraints were not, and both are now written down: the coordinated
Repl.Core/Repl.Defaults upgrade, with the one-directional compatibility that makes
it a constraint (docs/architecture.md), and the unconditional isHidden /
isAutomationHidden export fields that a closed schema has to allow
(docs/commands.md).
- The restore recipe promised more than it delivers. It restores the *code* for
paths that were already refusals, but not a path whose classification changed —
and one did: `tool --output:bogus` used to print help and exit `0`, is now a
UsageError, and the recipe makes it `1`. docs/configuration-reference.md says so,
and names the trade if that single path matters more than the distinction.
Summary
Testing
|
Rebased onto the merged #80, which had added three CHANGELOG sections after this branch was written. Taking the deletion is right — nothing reads the file and the release job generates versioned notes — but deleting it unaudited would have lost what only it recorded, which is the mistake the review caught on this PR the first time. #80 documented its rule well: docs/configuration-reference.md already covers the run-scoped token, the mode table, the epoch machine and the platform matrix. What lived only in the changelog was the *migration consequence* for an existing app, so that moves next to the rule it belongs to: - taking signal ownership by default under UseCliProfile/UseDefaultInteractive, and that ProcessSignalHandlingMode.None restores the previous behaviour; - interruption now resolving to 130/143 where OS-default termination used to apply, and what that does to a script treating any non-zero code as failure; - the handler-token change and how it fails — ObjectDisposedException from Register or WaitHandle, and IsCancellationRequested silently continuing to report false, which the existing "must not retain it" sentence did not say. That last one is the reason this audit was worth doing rather than assumed: a rule tells a reader what to do, and only the consequence tells them what breaks if they did not.
carldebilly
force-pushed
the
dev/cdb/remove-changelog
branch
from
September 10, 2026 18:51
b65a7fd to
504eded
Compare
carldebilly
added a commit
that referenced
this pull request
Sep 10, 2026
This page is what AGENTS.md points at for release guidance, and #88 made it the authority on release notes, so its claims were checked line by line against .github/workflows/ci.yml and version.json. Three were wrong and one large fact was missing. Corrected: - The stated prerelease pattern was `0.11.0-dev.{height}`; version.json says `0.12.0-dev.{height}`. The page no longer quotes the value at all — nbgv rewrites it at every release, so quoting it guarantees it goes stale — and points at the field instead. Examples that read "for example" are left as examples. - The page contradicted itself about release branch names: `release/<version>` with `release/0.10.0` in one section, "can be release/0.9 or release/1.0" in another. version.json's release.branchName is `release/{version}` and all five existing branches carry three-part versions, so the second form was simply wrong. - publicReleaseRefSpec was described as covering main and release/*; it also covers `v<major>.<minor>` tags. The page now also records that CI passes PublicRelease=true only for pushes to main and release/**, which is why a pull-request build produces .g<sha> packages that are never published. Added, because it is the most likely way a future release goes wrong: a release branch has no `{height}`, so every commit on it computes the same version. The two publish steps then disagree — `dotnet nuget push` runs with --skip-duplicate and tolerates it, while `gh release create` does not, so a second push to a release branch fails that step and, because Publish to NuGet is gated on `if: success()`, publishes nothing. A follow-up commit on a release branch therefore turns CI red with nothing wrong in the code. The page now says so and gives the way through. Also recorded that the release job downloads the packages artifact rather than rebuilding, so a release ships exactly what CI tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This page is what AGENTS.md points at for release guidance, and #88 made it the authority on release notes, so its claims were checked line by line against .github/workflows/ci.yml and version.json. Three were wrong and one large fact was missing. Corrected: - The stated prerelease pattern was `0.11.0-dev.{height}`; version.json says `0.12.0-dev.{height}`. The page no longer quotes the value at all — nbgv rewrites it at every release, so quoting it guarantees it goes stale — and points at the field instead. Examples that read "for example" are left as examples. - The page contradicted itself about release branch names: `release/<version>` with `release/0.10.0` in one section, "can be release/0.9 or release/1.0" in another. version.json's release.branchName is `release/{version}` and all five existing branches carry three-part versions, so the second form was simply wrong. - publicReleaseRefSpec was described as covering main and release/*; it also covers `v<major>.<minor>` tags. The page now also records that CI passes PublicRelease=true only for pushes to main and release/**, which is why a pull-request build produces .g<sha> packages that are never published. Added, because it is the most likely way a future release goes wrong: a release branch has no `{height}`, so every commit on it computes the same version. The two publish steps then disagree — `dotnet nuget push` runs with --skip-duplicate and tolerates it, while `gh release create` does not, so a second push to a release branch fails that step and, because Publish to NuGet is gated on `if: success()`, publishes nothing. A follow-up commit on a release branch therefore turns CI red with nothing wrong in the code. The page now says so and gives the way through. Also recorded that the release job downloads the packages artifact rather than rebuilding, so a release ships exactly what CI tested.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87a0a75f54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A release branch's version has no {height}, so every commit on it computes the same
number and `gh release create` failed on the second push — taking Publish to NuGet
with it, since that step is gated on `if: success()`. A follow-up commit on a
release branch therefore turned CI red with nothing wrong in the code.
The step now checks whether the release exists and re-attaches the packages with
`gh release upload --clobber` instead of recreating it, so the run stays green.
Deliberately not silent, and this is the part worth arguing. A published NuGet
version is immutable: if that follow-up commit changed shipped code, the packages
are skipped and the fix reaches nobody under that version. Simply turning the build
green would trade a false failure for a false success, which is worse. So the repeat
path emits a workflow warning and a job-summary note saying the packages were
re-attached, that NuGet will skip them, and that a change consumers should receive
needs a version bump.
The existing tag is left alone on that path. Moving a published tag changes what an
already-released version points at, which is a decision to take deliberately rather
than a thing CI should do by itself.
Both branches were exercised offline with gh stubbed, and `set -euo pipefail` was
checked against the `[[ ... ]] && VAR=` line it now precedes — bash exempts the left
operand of an AND-list, so a stable version still reaches the create path. YAML
validated.
docs/publishing.md is rewritten to describe this rather than the failure it
replaced: repeating is fine, repeating does not ship anything, and a change that
consumers must receive needs a new version.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5053b98bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…cing flow
Two findings on the publishing audit, both against claims I wrote.
- "A release publishes exactly the packages CI tested" did not follow from the
workflow: Test ran with --no-build, Pack did not, so dotnet pack was free to
re-run Build and a regenerated assembly could have reached a package the tests
never exercised. Rather than weaken the claim, Pack now uses --no-build too, so
Test and Pack share one Build's outputs and the guarantee holds. Verified locally:
pack --no-build produces all 10 packages and 9 symbol packages with no errors, and
the pack step runs on every pull request, so CI exercises this immediately —
unlike the release job.
- The servicing guidance told a maintainer to bump `version` on the release branch,
which the review read as bypassing the required prepare-release flow. Checking
Nerdbank.GitVersioning's own versioning-workflow documentation, neither reading was
right: its "service a released version" flow works *on* the release branch, and
prepare-release from main cuts a new line rather than a patch. It also documents
prepare-release being run on a release branch to move its stability stage.
So the page now follows that flow, and explains why this repository needs one
extra step inside it: release.branchName is `release/{version}` and prepare-release
strips the prerelease tag, leaving no {height}, so `nbgv get-version` on a serviced
commit answers with the version already published. The bump is that gap, not an
improvisation — and the resulting branch/version mismatch is cosmetic, since
publicReleaseRefSpec matches any `release/*`.
…to it A review finding against my own idempotence change from earlier in this branch, and it is right: I conflated "idempotent" with "re-upload" when it means "no-op". Replacing a published release's assets while NuGet skips the same immutable version would leave a direct GitHub download and a NuGet install of one version number carrying different binaries, with the tag describing neither. The repeat path now does nothing at all: no upload, no tag move, warning and job summary only. The run still stays green so Publish to NuGet is not skipped for a run with nothing wrong in it, and the tag keeps pointing at the commit that produced the published packages — the only commit it can honestly describe. Dropping the upload also removes the failure mode entirely, since the path no longer mutates the release at all. One correction for the record: the finding attributed to `gh release upload --help` a statement that it deletes existing assets before uploading and that a failed upload loses the originals. That text is not in the help output (gh 2.73.0), which says only "Overwrite existing assets of the same name". The finding's substance did not depend on it and stands on its own. docs/publishing.md updated to describe the no-op and why immutability makes it the right behaviour.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b56ab29d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Of four findings this round, one held. The warning I added said "nothing this run built was published", which is false in one case: Publish to NuGet still runs on the repeat path, and --skip-duplicate leaves published versions alone but uploads a package NuGet does not yet have. That is deliberate — it is how a push whose NuGet upload failed part-way recovers — so the message is corrected rather than the behaviour. Skipping NuGet on an existing release, as the finding proposed, would trade a misleading sentence for a pipeline that cannot recover from a transient push failure without manual intervention. docs/publishing.md now states the same thing, including the consequence worth knowing: such a recovery uploads from the current commit while the release's assets and tag stay on the one that created them. Also made the AGENTS.md distinction explicit rather than implied, since it was read the other way twice: running `nbgv prepare-release` on a release branch to move its stability stage is that tool's documented behaviour, and AGENTS.md's rule concerns not cutting a release from a feature or pull-request branch. Cutting a new line still starts from `main`. The other three findings are answered on the PR rather than applied. Two rest on premises that do not hold — one cites `gh release create --help` for a statement about draft/upload/publish mechanics that the help does not contain, and one describes a notes-baseline selection that this repository's own v0.11.0 notes contradict, having compared against v0.10.0 rather than a newer prerelease.
…th time Five rounds of edits to the same paragraphs produced exactly the drift this page's subject is about, so reading it end to end was worth more than another patch. Three defects, all mine: - A direct contradiction. "What repeating does not do is publish anything. Neither NuGet nor the release accepts a second version's worth of packages" sat three lines below a bullet stating that NuGet does upload a package it is missing. The previous commit corrected the workflow warning and left the prose asserting the absolute it had just stopped claiming. - A sentence from the --clobber design that no longer exists: "it re-attaches identical packages". Nothing re-attaches anything; the release is left untouched. - An orphaned paragraph. "That includes the tag..." lost its antecedent when the servicing section was inserted between it and the repeat-path discussion. The region is now written once, as four claims that do not overlap: the release is left entirely alone including its tag; NuGet still runs and publishes precisely what it lacks, which is how a partial push recovers; therefore a change meant for consumers needs a new version; and what a release contains. The servicing flow and the build/pack guarantee follow as their own subsections rather than trailing paragraphs. Verified the remaining claims still match the workflow warning they describe.
…ad as conflicting Reading the whole page rather than the section I had been editing turned up a tension the review was probably reaching for without naming it: the opening said "run `nbgv prepare-release` from a clean `main` only", four paragraphs before a servicing section that describes running it on a release branch. The qualifier "for stable release preparation" made the two technically consistent, but "only" reads as global, and a page whose job is to keep a release from going wrong should not depend on a reader parsing that carefully. The opening now says what it governs — cutting a release line starts from `main` — names the two invocations that operate on an existing release branch, and points at the section describing them. Same rule, no longer misreadable.
carldebilly
force-pushed
the
dev/cdb/remove-changelog
branch
from
September 10, 2026 20:18
a9d9f79 to
e15d4c1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e15d4c10b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`gh release create` with assets is not a single API call. Its manual documents that it creates the release as a draft, uploads the assets, then publishes it, so a run interrupted mid-upload leaves an unpublished draft behind — and `gh release view` finds it, because its lookup queries drafts alongside published releases. The existence check read that as a finished release, took the no-op path, and let `Publish to NuGet` push packages under a version with no release anyone could see. The check now tells three states apart through a single `--json isDraft` call. A draft stops the job with an error annotation and a job summary, publishing nothing. A published release is still left untouched, assets and tag. A lookup that fails for any other reason reads as missing and falls through to `gh release create`, which then fails loudly rather than skipping anything. Detection only, deliberately: publishing the draft and deleting it are both decisions about what has already reached consumers, and this job is the one part of the workflow that pull-request CI never exercises, so untested recovery logic here would be worse than a stop.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7a2250ab2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`Shell Completion Smoke (Real Shells)` failed with `curl: (22) The requested URL returned error: 500` and no retry line in the log. Exit 22 is curl's own, so the failure was not the `gh_api` helper — that one retries three times and returns 1. It was the asset download, the single curl in the step with no retry, and the one whose host actually returned the 500. curl counts HTTP 500 among its transient errors, so plain `--retry` covers this without `--retry-all-errors`. Confirmed against a local server that always answers 500: without the flag curl makes one request and exits 22, with `--retry 3` it makes four and then exits 22 — the same signature the failed job logged.
The servicing flow said "merge or cherry-pick the fix and commit it", and the release-notes section said `--generate-notes` builds the body from merged pull requests. Both are true, and together they describe a patch release that says nothing: a cherry-pick reaches the branch outside a pull request, so it contributes no entry, and this branch is the one that deleted CHANGELOG.md. Measured rather than assumed. `POST /releases/generate-notes` for the twelve commits between v0.12.0-dev.45 and this branch head — real commits, no merged pull request, since #88 is still open — returns a body of one line, the compare link, with no `What's Changed` heading at all. The same call across a range containing #80 returns its title, author and link. So the servicing section now asks for a pull request targeting the release branch and says why, with editing the release body by hand as the fallback when a fix lands as a direct commit; the workflow passes `--generate-notes` unconditionally and cannot supply notes for that path. The release-notes section states the empty case alongside what it already said about PR descriptions.
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
1 participant
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.
Why
CHANGELOG.mdarrived as a side effect of a feature PR (fde24c4, the hidden-option work) rather than as a release-process decision, and nothing consumes it — no workflow, no script, no packaging step, and no document links to it (grep -rn CHANGELOGoutside the file itself returns nothing).Meanwhile CI already publishes what it was trying to be:
Every published version gets notes generated from the merged pull requests, anchored to the version a consumer actually installs. The file could never be anchored that way — its own header said so, because Nerdbank.GitVersioning assigns the version at pack time — and a review lens on #85 correctly flagged that a breaking change pinned to "the commit closing issue #81" is not something a consumer whose CI broke on exit
2can search for.It was also the single point of conflict between concurrent pull requests, which is how the question came up: the only conflicting file when #85 merged, and again when #80 was rebased, with three more PRs open behind them.
What moves, and what was already covered
Only one piece of guidance lived exclusively here, and it is migrated:
docs/configuration-reference.md, beside theExitCodeOptionstable it talks about.The rest was already in the topic pages, so nothing is duplicated:
isHidden/isAutomationHiddenexport fields →docs/commands.mdCommandExecution.ExitCodefollowing the configured policy →docs/testing-toolkit.mddocs/publishing.mdgains a short Release notes section stating where notes come from and what that asks of a PR description, so the next person does not recreate the file.The
--targetfix is not theoreticalA review comment on this PR pointed out that
gh release createran without--target, and that gh creates a missing tag from the latest state of the default branch. The release job also triggers onrelease/**. Checking the repository's own history, that has already happened — four of the five stable tags point at a commit onmainand not at the release branch that could have produced their packages:release/**branchmainv0.9.0v0.9.1v0.9.2v0.10.0v0.11.0docs/publishing.mdconfirms CI is the tag's creator — "On push tomain/release/*, CI creates a GitHub Release with tagv<NuGetPackageVersion>" — so this isgh release create's automatic-tag path rather thannbgv. And sinceversion.jsononmainis0.12.0-dev.N, a stable version such as0.11.0can only have been packed fromrelease/0.11.0. Those releases therefore attach packages built on a release branch while tagging amaincommit, which makes both the tag's source snapshot and its generated comparison range wrong.One honest limit: this shows the tags are not on the branches that could have built them; it does not prove which commit produced each package. The inference rests on the stable version number, which is solid but is an inference.
The one-line
--target "${GITHUB_SHA}"change stops it recurring. Existing tags are left alone — moving published tags is a separate decision, and not one to fold into a docs PR.Sequencing
This conflicts with #80, which still carries
CHANGELOG.mdedits. Whichever merges second needs a rebase that drops those hunks — a deletion-vs-modification conflict resolved by taking the deletion. Merging this one second is the smaller of the two.Validation
markdownlint-cli2 "docs/**/*.md"— 0 issues in 28 filesgit diff --checkclean