mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
YuliiaKovalova
da5e63e878
* [dotnet-msbuild] Fix AP-13/AP-14 false positives and add source-vs-packed layout guidance The msbuild-quality-review run on microsoft/testfx surfaced two confidently-stated false positives that the maintainer rightly pushed back on: 1. The reviewer claimed unguarded `<Import>` forwarders in MSTest's NuGet `build/<tfm>/` folders point at non-existent paths under `buildTransitive/<tfm>/`. In reality those folders are produced at pack time by the `.nuspec` `<file target="...">` mappings, so the forwarders resolve correctly in every restored package. 2. The reviewer flagged backslashes in `<Import Project="...\...\...">` as a cross-platform 🔴 error. MSBuild's evaluator normalizes `\` to `/` on Unix-like systems via `FileUtilities.MaybeAdjustFilePath` / `ConvertToUnixSlashes` before resolving the path, so existing backslash-style imports work everywhere — confirmed by years of shipping MSTest to Linux/macOS users. This change updates the rubric so the same false positives don't recur across every consumer of the dotnet-msbuild plugin: * `msbuild-antipatterns/SKILL.md` AP-13 — adds an explicit "NuGet package forwarders" exception and tells reviewers to consult `.nuspec` / `<PackagePath>` before flagging. * `msbuild-antipatterns/SKILL.md` AP-14 — rewritten to distinguish where backslashes are a real bug (raw `<Exec>` shell strings, CDATA, non-MSBuild consumers — keep 🔴) from where they're only style (`<Import>` and other evaluator-routed paths — 🔵 with a cite to the MSBuild source). * `extension-points/SKILL.md` — adds a "Source Tree vs Packed Layout" section documenting the three packaging mechanisms (.nuspec `<file>` mappings, csproj `<PackagePath>` metadata, SDK pack conventions) that legitimately reshape the layout, with the cross-check procedure reviewers must run before flagging "missing-file" imports. * `agents/msbuild-code-review.agent.md` — Discovery now records the projected packed layout; Category 4 references the new AP-13/AP-14 nuances; new "Veracity gate" step downgrades or drops 🔴 findings that would imply currently-shipping CI is broken. No fixtures change: the `tests/dotnet-msbuild/msbuild-antipatterns/eval.yaml` rubric does not enumerate AP-13/AP-14 by id, and the F#-specific scenarios are unaffected. Markdownlint passes on all edited files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review feedback - extension-points: rewrite .nuspec example so the source file is genuinely shared (single buildTransitive/common/MyAdapter.props fan-out instead of three already-per-TFM sources); clarify <file target=> folder-vs-rename semantics. - extension-points: replace semicolon-list PackagePath example with the unambiguous multi-<None> form, and note in prose that semicolon syntax is also supported. Avoids reader confusion noted in review. - msbuild-antipatterns AP-13 cross-check: align search scope with extension-points (project directory AND any parent directory) so reviewers don't miss shared mono-repo nuspecs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * AP-13: drop ../ link to extension-points to satisfy skill-validator Skill-validator rejects file references that traverse out of the skill directory (parent-directory traversal). Replace the markdown link with a plain-text reference to the dotnet-msbuild/extension-points skill — the prose is still discoverable to humans and to the agent that loads the plugin, but no longer trips the validator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address second round of PR review feedback - AP-13: replace <tfm> placeholders inside XML attribute strings with concrete TFM examples (net8.0) so the snippets are valid copy-pasteable XML. - AP-13 & extension-points: bound the .nuspec search to project directory + IMMEDIATE parent only (no unbounded walking); wording now identical in both docs. - AP-14: add explicit text severity next to severity emoji (🔴 Error / 🔵 Style) for accessibility — screen readers and plain-text copies now convey severity without relying on color/emoji rendering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>