Skip to content

docs: ADR 0002 pack gate + trim Directory.Build narration - #785

Merged
tylerkron merged 2 commits into
mainfrom
cursor/adr-0002-pack-gate-8b10
Sep 27, 2026
Merged

tylerkron merged 2 commits into
mainfrom
cursor/adr-0002-pack-gate-8b10

Conversation

@tylerkron

@tylerkron tylerkron commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

ADR 0002 says that appending a parameter to a public positional record is not a breaking change. It never says that dotnet pack still fails when you do it. The next contributor who appends a record field would hit CP0002 from package validation with nothing in the ADR explaining it. Separately, the comments in .editorconfig, Directory.Build.props and Directory.Build.targets had grown into project history (file counts, "Daqifi.Mcp never opted in", "#644 shipped with none of this"), which buried the reasons a maintainer actually needs.

This PR changes only the ADR and comments. It changes no analyzer severity, TFM, MSBuild property or build logic. ci.yml, DEVICE_INTERFACES, CONTRIBUTING.md, and the packed README consumer policy are untouched.

  • ADR 0002, new "Pack gate" consequence. A PR that appends a positional-record parameter fails dotnet pack with CP0002 for the old-arity constructor and Deconstruct (both TFMs) against the nuget.org baseline. That is expected under this decision. The same PR treats it as the intentional break CONTRIBUTING.md ("The published package is the second opinion") describes: remove the old signatures from PublicAPI.Shipped.txt and check in the generated CompatibilitySuppressions.xml. The suppressions must then be removed in the change that bumps PackageValidationBaselineVersion, because a stale suppression fails the pack as "Unnecessary suppressions found".
  • .editorconfig. Drops the 142-vs-~200 namespace-style story and the lists of files. Each rule keeps its name and reason:
  • Directory.Build.props. Drops the "Daqifi.Mcp never opted in" note and the chore(build): decide whether Daqifi.Mcp should multi-target net9.0;net10.0 like Daqifi.Core #643 TFM-asymmetry story. Keeps "do not set TargetFramework here" and the TargetFrameworkTests pointer.
  • Directory.Build.targets. Drops "Daqifi.Mcp shipped with none of this (chore(build): the published Daqifi.Mcp package has no license, project URL, repository URL, tags, or SourceLink #644)" and shortens the EmbedUntrackedSources paragraph to one line: it restates an SDK default on purpose, so nobody removes it as redundant. Keeps the IsPackable import-order note, the coverlet/ContinuousIntegrationBuild warning, and the explicit Company comment.

Verification

  • A script compares origin/main with this branch: the .editorconfig non-comment lines are identical, and the comment-stripped C14N XML of both Directory.Build.* files is identical. So no values, conditions or severities changed.
  • The ADR claims were checked against a real pack. I appended a probe parameter to ChannelAcquisitionStatistics: CP0002 fired for the ctor and Deconstruct on net9.0 and net10.0. -p:ApiCompatGenerateSuppressionFile=true wrote src/Daqifi.Core/CompatibilitySuppressions.xml, and the next pack consumed it automatically. After I reverted the probe, that file failed the pack as unnecessary. Nothing from the probe is committed.
  • The CA1707 comment was checked by building with api_surface = all. Only a private static readonly _camelCase field and seven private SCREAMING_SNAKE consts were reported; instance _camelCase fields were not.
  • dotnet build Daqifi.Core.sln is clean, and dotnet pack src/Daqifi.Core/Daqifi.Core.csproj passes on this tree.
Open in Web Open in Cursor 

🤖 Generated with Claude Code

Appending a positional-record parameter removes the old constructor, so
package validation fails CP0002 until a suppression is checked in and
later deleted with the baseline bump. Drop incident history from the
repo-shell comments and keep the constraints those comments exist for.

Co-authored-by: Tyler Kron <tylerkron@gmail.com>
@tylerkron
tylerkron requested a review from a team as a code owner September 24, 2026 10:42
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Document package-validation gate and streamline build comments

📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Documents CP0002 suppression lifecycle for source-compatible positional-record additions.
• Condenses analyzer and shared-build comments while preserving current constraints.
• Leaves analyzer severities, target frameworks, and build behavior unchanged.
Diagram

graph TD
  A["Record append"] --> B["Package validation"] --> C["CP0002 failure"] --> D["ADR policy"] --> E["Contributing guide"] --> F["Add suppression"] --> G["Baseline bump"] --> H["Remove suppression"]
Loading
High-Level Assessment

The current approach is appropriate: record the compatibility exception in the existing ADR, reference the established contributor workflow, and avoid changing build behavior. Adding a permanent compatibility shim or modifying package-validation settings would conflict with the accepted source-compatibility policy.

Files changed (4) +22 / -70

Documentation (4) +22 / -70
.editorconfigCondense analyzer-scope rationale +8/-48

Condense analyzer-scope rationale

• Replaces historical analyzer narratives with concise explanations for namespace style, CA2007 and CA1707 scope, and the generated-file RS0041 exemption. Analyzer settings and severities remain unchanged.

.editorconfig

Directory.Build.propsSimplify shared build-property commentary +5/-15

Simplify shared build-property commentary

• Shortens the TargetFramework guidance while retaining project-local framework ownership and the framework-alignment test reference. Removes historical narration around repo-wide warning enforcement without changing properties.

Directory.Build.props

Directory.Build.targetsTrim package metadata history +1/-7

Trim package metadata history

• Removes incident history from the shared package-metadata explanation and deletes redundant EmbedUntrackedSources default narration. All MSBuild property values and conditions remain unchanged.

Directory.Build.targets

0002-binary-compatibility-policy.mdDocument the package-validation suppression lifecycle +8/-0

Document the package-validation suppression lifecycle

• Explains that positional-record parameter additions produce CP0002 against the published baseline despite the accepted source-compatibility policy. Directs maintainers to add a compatibility suppression and remove it after the baseline includes the new constructor.

docs/adr/0002-binary-compatibility-policy.md

@qodo-code-review

qodo-code-review Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Package validation still fails on the documented break ✗ Dismissed 🐞 Bug ≡ Correctness
Description
The new ADR says the positional-record constructor removal triggers CP0002 and instructs
contributors to check in a CompatibilitySuppressions.xml, but this PR adds no such suppression.
Daqifi.Core.csproj enables package validation against version 1.8.0, so packing the current API
continues to fail until the documented suppression is actually present.
Code

docs/adr/0002-binary-compatibility-policy.md[R85-91]

+**Pack gate**
+- Appending a positional-record parameter removes the old constructor, so
+  `EnablePackageValidation` fails the pack with `CP0002` against the nuget.org baseline.
+  That is expected under this decision; it is not a major-version bump. Suppress it the way
+  `CONTRIBUTING.md` ("The published package is the second opinion") describes — check in a
+  `CompatibilitySuppressions.xml` — and delete that suppression when
+  `PackageValidationBaselineVersion` bumps to a release that contains the new constructor.
Relevance

●●● Strong

Recent reviews accept enforcing package-validation baseline correctness and preserving compatibility
for positional-record changes.

PR-#705
PR-#321

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed ADR explicitly identifies CP0002 as a pack failure and says a
CompatibilitySuppressions.xml must be checked in. The project enables package validation with a
fixed 1.8.0 baseline, while repository search finds no compatibility suppression file or suppression
configuration, leaving the stated failure unaddressed.

docs/adr/0002-binary-compatibility-policy.md[85-91]
src/Daqifi.Core/Daqifi.Core.csproj[20-42]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR documents that appending a positional-record parameter removes the old constructor and causes CP0002 against the 1.8.0 package-validation baseline, but the repository contains no `CompatibilitySuppressions.xml`. `Daqifi.Core.csproj` enables package validation, so `dotnet pack` continues to fail on the documented compatibility break.

## Fix Focus Areas
- docs/adr/0002-binary-compatibility-policy.md[85-91]
- src/Daqifi.Core/Daqifi.Core.csproj[20-42]
- src/Daqifi.Core/CompatibilitySuppressions.xml[1-20]

## Recommended Fix
Generate the ApiCompat suppression for the intentional removed constructor, check the resulting `src/Daqifi.Core/CompatibilitySuppressions.xml` into the repository, and ensure the package-validation target consumes it. Keep the ADR instruction to delete the suppression when `PackageValidationBaselineVersion` is bumped to a release containing the new constructor; verify with `dotnet pack src/Daqifi.Core/Daqifi.Core.csproj` without `--no-build`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 13 rules
✅ Cross-repo context — repo relationships
Review mode: 🚀 Fast: The push is limited to documentation and explanatory comments with no runtime, build, configuration, or schema behavior changes.

Grey Divider

Tip of the day
💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread docs/adr/0002-binary-compatibility-policy.md Outdated
…trimmed reasons

ADR 0002's pack-gate note read as if the current tree already fails
package validation. It does not: the note is about a future PR that appends
a positional-record parameter. Reworded to say so, and corrected against an
actual pack with a probe parameter appended: CP0002 fires for the old-arity
constructor and Deconstruct on both TFMs, and a stale suppression fails the
pack with "Unnecessary suppressions found", so removing it at the baseline
bump is required.

Restored the load-bearing reasons the trim lost: the rule names and the
every-await scope for CA2007, the precise CA1707 api_surface=all fallout
(private static readonly _camelCase fields, verified by building with it),
why the generated file cannot be annotated for RS0041, and why
EmbedUntrackedSources restates an SDK default. Comments only; no setting,
severity or MSBuild value changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2ddf907

@tylerkron

Copy link
Copy Markdown
Contributor Author

Qodo-clean, CI green — ready for review

@tylerkron
tylerkron added this pull request to the merge queue Sep 27, 2026
Merged via the queue into main with commit 88e20de Sep 27, 2026
4 checks passed
@tylerkron
tylerkron deleted the cursor/adr-0002-pack-gate-8b10 branch September 27, 2026 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants