docs: add CLAUDE.md with the working agreement - #96
Conversation
Split out of #93, where it was committed only because a repository hook requires a clean working tree and this session could not create a second branch at the time. Copilot flagged the mixed scope there, correctly, and the file's own grouping rule says the same. Records what had so far only been said in conversation, so the next session starts from it instead of rediscovering it: * every change lands through a pull request, and tickets that touch the same files in the same package share one -- two pull requests racing over `CanBusService.cs` cost more review than they save; * the repository squash-merges, so the pull-request title is the commit semantic-release reads, and a `BREAKING CHANGE:` footer has to live in the pull-request body to survive that; * a .NET 10 SDK is installable here from the Ubuntu archive even though `builds.dotnet.microsoft.com` is blocked by the proxy, so there is no reason to push a guess and use CI as the compiler -- with the full local gate (build with CI=true, test, format, pack + verify-packages); * API approval baselines come from the generated `.received.txt`, and XML and YAML get a well-formedness check after editing (a `--` inside an XML comment once made every project fail to load); * the pre-1.3.0 versioning rules, and that upstream CanKit types are read at the pinned tag rather than assumed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
PR SummaryLow Risk Overview Corrects release documentation that previously described squash merges: Reviewed by Cursor Bugbot for commit 089f669. Bugbot is set up for automated code reviews on this repo. Configure here. |
Copilot's scope objection on this pull request was correct: the working agreement is unrelated to the raw-CAN change, and the file's own grouping rule says so. It was here only because a repository hook requires a clean working tree and this session could not create a second branch at the time. That constraint is lifted, so the file now lives in #96 and this pull request is back to one subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 359e550f16
ℹ️ 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".
Codex found that `CONTRIBUTING.md`'s central release claim is false, and I had
copied it into CLAUDE.md unverified. Checked against the history rather than
argued about:
$ git log --first-parent --oneline origin/main | head -3
52834b2 Merge pull request #91 from dborgards/claude/sort-open-issues-v9a7kk
bd729e3 Merge pull request #90 from dborgards/test/dispose-hygiene
c2712ba Merge pull request #89 from dborgards/chore/format-and-gate
Every one has two parents. Nothing is squash-merged; every branch commit is
retained, and semantic-release analyses all of them. So the pull-request title
is not "the commit semantic-release reads" -- the branch commits are.
The practical inversions:
* a `docs:`-titled pull request containing one `feat:` commit publishes a minor
release, which the old guidance would have called impossible;
* a `BREAKING CHANGE:` footer belongs in the commit message, where the analyser
reads it. Putting it only in the pull-request body -- which is what I did on
#93, and told the author was necessary -- does nothing. No harm there, since
the commit carried it too, but the reasoning was wrong.
Both documents now describe the observed behaviour. CONTRIBUTING.md is
corrected at the source rather than left contradicting the file derived from
it; that is one file beyond this pull request's nominal scope, and leaving the
original assertion standing would have been worse than the scope creep.
Neither document decides the strategy. If the intent is squash merging, that
belongs in the repository settings, and then both texts want revisiting --
noted in CLAUDE.md for the owner rather than assumed either way.
Also from the same review: the packaging gate used a fixed `/tmp/nupkgs`.
Re-running it leaves earlier artifacts in place, and since `verify-packages.py`
checks whatever `*.nupkg` it finds, a change that stops producing a package
could pass on the stale copy. Now packs into a fresh `mktemp -d`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f41d4652e4
ℹ️ 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".
f41d465 claimed to fix this and did not. The script that made the change ran two replacements but built the second write from the *original* file contents, so writing the packaging fix silently reverted the merge-strategy fix. Only CONTRIBUTING.md ended up corrected, while CLAUDE.md kept asserting the opposite -- and I reported the fix as done on the review thread. Cursor Bugbot caught the contradiction between the two files; I did not, having verified neither the file nor my own claim after writing it. Verified this time by grepping the result rather than trusting the script's exit status. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
Third place carrying the same false claim, found by Codex after the first two
were fixed: the template told contributors "the merge commit is what
semantic-release reads to decide the next version", so a valid title was
presented as sufficient. With merge commits it is not -- every retained branch
commit is analysed, and a `docs:`-titled pull request containing one `feat:`
commit publishes a release.
The template is the copy contributors actually read, so leaving it while
correcting CONTRIBUTING.md and CLAUDE.md would have fixed the two documents
nobody opens while writing a pull request.
Verified that no variant of the claim survives anywhere:
$ grep -rni "title.*semantic-release reads|squash-merged" \
.github/PULL_REQUEST_TEMPLATE.md CONTRIBUTING.md CLAUDE.md
(no matches)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 089f669285
ℹ️ 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".
| release. A breaking change needs its `BREAKING CHANGE:` footer in the *commit message*, where the | ||
| analyser reads it; the PR body is not analysed. See CONTRIBUTING.md. Examples: |
There was a problem hiding this comment.
The revised guidance points contributors to the examples, but the very next example still says a breaking commit “bumps the major,” and the linked example in CONTRIBUTING.md says the same. While v1.3.0 has not been released, .releaserc.json explicitly maps breaking: true to minor, as the new CLAUDE.md also explains, so contributors are given the wrong release expectation during the current versioning window; qualify these examples or label the bump as minor until that tag exists.
Useful? React with 👍 / 👎.
Two findings from the review of this branch, both caused by extending the chronology from eight rows to eleven without re-reading what referred to it: - The header still said eight merged pull requests while the inventory two paragraphs below listed nine (#93, #96, #97, #98, #100, #101, #104, #107, #108). Corrected to nine. - A blank line between row 8 and row 9 terminated the Markdown table, so rows 9-11 rendered as plain pipe-delimited text. Removed. Two more of the same class that the review did not name: "in jedem der acht Faelle" in the cause section refers to the measurement failures only, so it is now "der ersten acht"; "von den acht Zeilen oben" means the whole table and is now "elf". The section on cross-paragraph contradictions records this occurrence, since the document reproduced the very error class it describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
What does this change?
Adds a root
CLAUDE.mdrecording the working agreement that has so far only existed in conversation: pull-request and grouping rules, the squash-merge footer trap, the local verification gate, the pre-1.3.0 versioning rules, and how to read upstream CanKit types.Split out of #93, where it did not belong. It was committed there only because a repository hook requires a clean working tree and that session could not create a second branch at the time; Copilot flagged the mixed scope, correctly, and the file's own grouping rule says the same. #93 drops it in the same round, so nothing is duplicated between the two.
Type of change
feat— new behaviour (minor release)fix/perf— bug or performance fix (patch release)docs/test/refactor/chore/ci— no releaseChecklist
dotnet build CanKit.Pro.sln -c Releasesucceeds — unchanged; this PR adds one Markdown file at the repository root and touches no project, source ordocs/contentdotnet test CanKit.Pro.sln -c Releasepasses — as aboveThe two entries worth reading before the rest
The SDK is installable here.
builds.dotnet.microsoft.comis blocked by the proxy, which makes it look as though no .NET SDK can be had and CI must serve as the compiler. It is not so —dotnet-sdk-10.0is in the Ubuntu archive:apt-get install -y --no-install-recommends dotnet-sdk-10.0 # noble-updates, 10.0.112An earlier session spent a long time working around a constraint that did not exist. This is the single most useful line in the file.
The squash-merge footer trap.
CONTRIBUTING.mdsays the PR title becomes the commit semantic-release reads. It does not say that aBREAKING CHANGE:footer living only in a commit message may therefore not reach the changelog. On #93 that would have shipped 1.3.0 announcing a break without saying what to do about it.The remaining sections — grouping, the local gate, API approval baselines, XML/YAML well-formedness, reading upstream types at the pinned tag — each come from a mistake made in this repository, not from general advice.
Scope
One new file. No project file, no source, no test, no
docs/page (so the MkDocs nav is untouched and--strictis unaffected). It carries no release effect: the title isdocs:.🤖 Generated with Claude Code
https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
Generated by Claude Code