Uh oh!
There was an error while loading. Please reload this page.
Warn that adding a reporter file needs a dotnet/sdk manifest append - #10402
Conversation
The reporter Include is a glob, so a new partial joins the shared-source contract automatically and the build stays green. dotnet/sdk's eng/vendored-files.json enumerates each upstream path by hand, so a newly added file is invisible to its drift detector - later edits to it raise no drift issue at all. Renames and deletions are caught (the path 404s), which makes additions the one silent case. Splitting TerminalTestReporter.Summary.cs into partials hit exactly this (#10390), so record it where a contributor adding a file will see it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8858a93-4a8d-4028-b0db-c221ef158206
There was a problem hiding this comment.
Pull request overview
Documents the required downstream manifest update when adding terminal reporter source files.
Changes:
- Explains why new files are invisible to the SDK drift workflow.
- Records the append-only manifest rule.
Show a summary per file
| File | Description |
|---|---|
TerminalReporterContract.props | Adds contributor guidance for coordinated SDK manifest updates. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
There was a problem hiding this comment.
i️ Copilot Review · #10402
Summary: Comment-only addition documenting the asymmetric failure mode when adding files to the reporter contract folder. No code, build, or API changes.
Verdict: No issues found across all 22 review dimensions. The new comment is accurate, actionable, and well-placed — it explains the silent failure mode (additions invisible to dotnet/sdk drift detection), gives the correct mitigation (append to manifest), and warns about the reordering pitfall with a concrete reference (#10390).
No concerns with XML validity (-- not present inside the comment delimiters), encoding, or whitespace.
Uh oh!
There was an error while loading. Please reload this page.
Records, in the file testfx contributors actually touch, that adding a reporter source file needs a coordinated
dotnet/sdkmanifest change. Follow-up to #10387 / #10390. Comment-only — no build behavior changes.Why
TerminalReporterContract.propsalready tells contributors that the reporter is hard-forked intodotnet/sdkand that "any change here may require a coordinated port". That covers edits. It does not cover additions, which behave differently and fail silently:Include="$(MSBuildThisFileDirectory)*.cs"picks up a new partial automatically, and the standalone contract test project keeps compiling. Nothing signals that anything is owed downstream.eng/vendored-files.jsonlists each upstream path by hand and the drift workflow only watches the paths it lists. So a file we add isn't merely unported — it's invisible: later edits to it raise no drift issue at all.That asymmetry is specific to additions. I read
check_vendored_files.pyto confirm the other cases are fine: a rename or delete makes the tracked path 404, which the script maps tostatus="missing"→ an "upstream path missing" issue. Additions are the one silent case.This is not hypothetical — it's precisely what #10387 caused, and it took a manual audit to notice. The note also records the append-never-insert rule, because the bot's issue identity is the source's array index (
vendored-sync:id={id}:{source-index}); sorting a new entry into the list silently re-points existing open drift issues at the wrong file. I hit that myself while fixing the manifest and only caught it by running the checker before and after.Note
While writing this I initially pasted the literal marker
<!-- ... -->into the XML comment, which terminated it early and made the file invalid XML. Caught by parsing the file; the committed version quotes the marker without the delimiters. Worth mentioning because MSBuild's own error for this is not obvious.Validation
[xml]parses the file, and no--sequence remains inside any comment block.Microsoft.Testing.Platform.TerminalReporterContract.UnitTests(the project that imports this props) builds and passes onnet8.0andnet9.0— 0 warnings.Related
The
dotnet/sdkhalf is dotnet/sdk#55564, which appends the four new partials to the manifest and documents the same append-only rule there. This PR is the mirror note on the source-of-truth side, so the requirement is visible before someone adds the next partial rather than after.