Scope VerifyReferenceAotCompatibility to an AOT Publish in dotnet-codestyle - #1861
Merged
Merged
Conversation
…estyle State IsAotCompatible unconditionally and VerifyReferenceAotCompatibility only under the PublishAot condition, with the reason: reference verification reports IL3058 for every dependency not built AOT-compatible, which TreatWarningsAsErrors turns into a failed build on ordinary fleet dependencies. Regenerate the skill distributions. Closes on promotion: #1857 Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Directory.Build.props is imported before the project body, so a condition on PublishAot there sees it empty and never enables reference verification. Name the .csproj or Directory.Build.targets instead. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new placement rationale for avoiding Directory.Build.props is slightly overstated and should be reworded for accuracy.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Updates the dotnet-codestyle skill documentation to scope VerifyReferenceAotCompatibility to AOT publishes (to avoid IL3058 dependency warnings becoming build failures under TreatWarningsAsErrors), and regenerates the distributed skill copies.
Changes:
- Document
IsAotCompatible=trueas unconditional, while movingVerifyReferenceAotCompatibility=trueunder aPublishAot-conditioned property group. - Add guidance on where to place the conditioned property group (
.csprojafter settingPublishAot, orDirectory.Build.targets). - Regenerate the
.github/skills/and.claude-plugin/distributions and update the source digest.
| File | Description |
|---|---|
| .agents/skills/dotnet-codestyle/SKILL.md | Updates the high-level summary line to reflect AOT-scoped reference verification. |
| .agents/skills/dotnet-codestyle/references/project-config.md | Documents the conditional VerifyReferenceAotCompatibility rule and placement guidance. |
| .github/skills/dotnet-codestyle/SKILL.md | Regenerated distribution copy of the updated skill summary. |
| .github/skills/dotnet-codestyle/references/project-config.md | Regenerated distribution copy of the updated project configuration reference. |
| .claude-plugin/fleet-skills/skills/dotnet-codestyle/SKILL.md | Regenerated plugin distribution copy of the updated skill summary. |
| .claude-plugin/fleet-skills/skills/dotnet-codestyle/references/project-config.md | Regenerated plugin distribution copy of the updated project configuration reference. |
| .claude-plugin/fleet-skills/.source-digests/dotnet-codestyle | Updates the source digest for the plugin distribution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A global -p:PublishAot=true is visible in Directory.Build.props, so the rationale now names only the case the placement rule guards: a PublishAot the .csproj sets. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The documentation change is internally consistent across sources and generated copies, and the revised Directory.Build.props rationale matches MSBuild import behavior without overstating it.
Review effort: Lite
Findings: None
Resolved since last review (1)
This was referenced Sep 26, 2026
ptr727
added a commit
that referenced
this pull request
Sep 26, 2026
Fixes a finding CodeRabbit raised on the promotion PR #1850, against #1861's text. `dotnet-codestyle`'s project-config reference said reference verification reports `IL3058` for every dependency "not built AOT-compatible", and named `System.CommandLine` and the Serilog sinks as failing. Microsoft's IL3058 page gives the cause as a referenced assembly without the `IsAotCompatible` assembly metadata set to `true`. That's a missing mark, not proof of incompatibility, and a library built with the property against a target before .NET 10 still lacks it. The unversioned package names also claim a state their newer releases may not have. The sentence now names the missing mark and drops the examples. Only the source under `.agents/skills/` was edited. The `.github/skills/` and `.claude-plugin/` copies and the source digest are regenerated by `build_dist.py`, and `--check` passes. The prose gate, markdownlint, and cspell pass. One local strict review pass is recorded, with no findings. It checked the wording against the Microsoft Learn pages for IL3058 and Native AOT. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ptr727
added a commit
that referenced
this pull request
Sep 26, 2026
…and Overnight Fixes to Main (#1850) Promotes `develop` to `main`. ## Carried - #1846: exits `repo-config/configure.sh` early, writing nothing, for a repository whose registry `status` is `archived`. - #1848: fails the validator's C# unit test step when the run wrote no non-empty Cobertura report, clearing `./coverage` first. It declines #1134's `ref` input with evidence, since a bare checkout already validates `github.sha`, and D1.2 now says so. #1134's third gap moved to #1800. - #1851: asserts that the two planted registration defects are themselves reported. - #1854, #1858: correct #1846's test docstrings, which claimed the script makes no `gh` call before the archived check in cases where it does. Raised by Copilot on this pull request. - #1856: passes `--repo` on documented handoff commands and guards handoff reads against a full page, per #1847. - #1859, #1863: fall back to the default search path when `PATH` is unset in #1848's and #1846's test harnesses. Raised by Copilot on this pull request. - #1861: scopes `VerifyReferenceAotCompatibility` to an AOT publish in `dotnet-codestyle`, per #1857. - #1867: establishes the `PATH` order `tool_shadow_path` names, per #1644. - #1870: makes the installer's dirty-checkout tests independent of the real checkout's state, per #1641. - #1873: pins and decodes git's quoting in `repo_gate.py`'s `ls-files` read, per #1580 and #1872. - #1878: folds typographic punctuation in `pr_review.py reply --match`, per #1299. - #1883: routes `configure.sh`'s `ruleset_id()` through `jqr`, per #1253. - #1885: states the pin comment as the release tag and defines `$/`, per #1805. - #1888: distinguishes `./` from `$/` resolution in the pin rule's prose, per #1886. - #1893: drops the issue reference from `repo-config/README.md`'s archived-exemption note, which Copilot flagged on six rounds of this pull request. - #1895: describes IL3058 in `dotnet-codestyle` as a referenced assembly lacking `IsAotCompatible` metadata set to `true`, and drops the unversioned package examples. Raised by CodeRabbit on this pull request. Callers that pin a hub release get the new C# check on their next pin bump. A test project that runs `dotnet test --coverage` without writing a report now fails its step rather than passing silently. Closes #1134 Closes #1847 Closes #1857 Closes #1644 Closes #1641 Closes #1580 Closes #1872 Closes #1299 Closes #1253 Closes #1805 Closes #1886 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Backlog counts and rankings now consistently exclude handoff issues, including those also marked blocked. * Repository configuration commands now exit without writing when a repository is archived. * Review-thread matching handles typographic punctuation, and no-match responses report the unresolved-thread count. * Tool setup handles PATH entries more precisely, and repository checks report unusual file paths without crashing. * **Reliability** * Validation now fails when C# or Python tests produce no coverage report. * Agent setup can use an explicit dirty-checkout override. * Workflow and repository guidance clarifies reference resolution, release-tag pinning, and AOT configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
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.
Summary
dotnet-codestylestatedVerifyReferenceAotCompatibilityas an unconditional project setting. Reference verification reportsIL3058for every dependency not built AOT-compatible, andTreatWarningsAsErrorsturns that into a failed build on ordinary fleet dependencies.references/project-config.mditem 2 now statesIsAotCompatibleunconditionally andVerifyReferenceAotCompatibilityonly inside aPublishAot-conditioned property group, with the reason..csprojafter it setsPublishAot, orDirectory.Build.targets, neverDirectory.Build.props, which is imported before the project body and would seePublishAotempty.SKILL.mdsummary line matches, and the.github/skills/and.claude-plugin/copies are regenerated byscripts/build_dist.py.Closes on promotion: #1857
Verification
scripts/build_dist.py --check,scripts/prose_lint.py --diff origin/develop,scripts/repo_gate.py,spec/validate.py, and markdownlint on the changed sources all pass.local-strict-reviewpasses recorded: the first raised theDirectory.Build.propsplacement trap, fixed in the second commit, and the second found nothing.🤖 Generated with Claude Code