Promote Develop to Main for the Hub Resync - #942
Merged
Conversation
#927) Bumps the actions-deps group with 1 update: [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv). Updates `astral-sh/setup-uv` from 9.0.0 to 10.0.1 - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@c771a70...20cfd1b) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 10.0.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions-deps ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps the actions-deps group with 1 update: [streetsidesoftware/cspell-action](https://github.com/streetsidesoftware/cspell-action). Updates `streetsidesoftware/cspell-action` from 8.4.0 to 9.0.1 - [Release notes](https://github.com/streetsidesoftware/cspell-action/releases) - [Changelog](https://github.com/streetsidesoftware/cspell-action/blob/main/CHANGELOG.md) - [Commits](streetsidesoftware/cspell-action@de2a73e...e0668cf) --- updated-dependencies: - dependency-name: streetsidesoftware/cspell-action dependency-version: 9.0.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions-deps ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps the actions-deps group with 1 update: [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action). Updates `docker/setup-buildx-action` from 4.2.0 to 4.3.0 - [Release notes](https://github.com/docker/setup-buildx-action/releases) - [Commits](docker/setup-buildx-action@bb05f3f...37fe631) --- updated-dependencies: - dependency-name: docker/setup-buildx-action dependency-version: 4.3.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions-deps ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…Chain (#940) * Adopt the fleet LF line-ending default and renormalize the tree The hub moved the fleet default from CRLF-with-LF-exceptions to LF-with-a-CRLF-exception-for-bat-and-cmd. `.editorconfig` now declares `[*] end_of_line = lf` and `.gitattributes` declares `* text=auto eol=lf`, so git enforces the ending on checkout and renormalize rather than leaving it to the editor. The per-type LF pins those two files carried (`*.sh`, the husky hook, `uv.lock`, Dockerfiles, workflow YAML) are redundant under the new default and are dropped. `git add --renormalize .` converts the 160 tracked CRLF files to LF. Only `.editorconfig`, `.editorconfig-checker.json` and `.gitattributes` carry content changes; every other file in this commit is line-endings only, as `git diff --ignore-cr-at-eol` shows. The six binary files are untouched. * Re-vendor the carried instruction set from the hub Carries `CLAUDE.md`, the file that imports `AGENTS.md` into a Claude Code session, which this repo never had. Re-vendors the verbatim sections the audit reported stale: `AGENTS.md` "Context and Delegation Discipline" and "Where the Rules Live", and `GOVERNANCE.md` "Durable Knowledge and Self-Improvement", "Repository Boundaries and Write Safety", "Operational Repositories", "Hub-Hosted Tooling", "Verification Discipline", "PR Review Etiquette", "Workflow YAML Conventions" and "Repository Details". The repo-specific intro paragraphs, "Devcontainer" and "Repository Layout" are intent-fidelity and stay this repo's own. "Running the Linters Locally" is promoted from `###` to `##`, matching the hub's own heading level so the `AGENTS.md` rules table resolves, and takes the hub's current content: the per-tool Docker one-liners are superseded by the hub-hosted `scripts/docker_lint.py` wrapper. The PowerShell block goes with them, since this repo tracks no `.ps1` file and the wrapper discovers its own targets. `CODESTYLE.md` takes the hub's current local-gate and Shell wording, and `AGENTS.md` now names the `shell-codestyle` Skill alongside the other two. The PlexCleaner .NET and Python subsections are unchanged. `AUDIT.md` is rewritten against hub-hosted configuration: `repo-config/` and `spec/secrets.json` are no longer carried, so the self-audit reaches `repo-config/configure.sh check` and `spec/audit.py` from a hub checkout rather than diffing local payloads that are about to be deleted. `.github/copilot-instructions.md` takes the hub's current runbook, which routes to `scripts/pr_review.py` in place of the hand-rolled GraphQL. The "Disproved Claims" section keeps its shape and rules and carries no entry: this repo had none of its own, and the hub's are proofs about the hub's tree. * Delete the retired repo-config and spec carries `spec/divergences.json` gives both trees a `retire` disposition: the branch rulesets, the repository settings and the secrets declaration are hub-hosted ground truth reached from a hub checkout, not payloads a repo carries. A carried copy can only restate the hub's computation or drift from it between resyncs. The inbound sweep re-points the two "Repository Layout"/"Configuration Layout" bullets that described the directories, and drops the `host-tools.json` note's aside citing the carried `spec/secrets.json` as precedent for omitting a `$schema` pointer. `AUDIT.md` was already rewritten against the hub-hosted payloads in the previous commit. The `HISTORY.md` mention stays: it records what a past release shipped and was true then. * Carry the .github/skills tree from the hub `spec/files.json` declares `.github/skills` a `verbatim-tree` with `prune`, applying to every fleet repo, so the 37 files are carried whole rather than adapted. `scripts/carry.py apply` wrote them and reports the source and target digests equal. `.github/copilot-instructions.md` already points a review at `.github/skills/code-review/SKILL.md`; until now that path resolved to nothing here. * Migrate the test project to Microsoft.Testing.Platform The .NET 10 SDK refuses to run a VSTest target for a project that pulls in `Microsoft.Testing.Platform.MSBuild`, which xunit.v3 4.0.0 does. That is what has been failing the `Run unit tests` and `Build Docker image` jobs on Dependabot's nuget-deps bumps (#937 to `develop`, #938 to `main`), each re-rolled once with the same result, so the group has not landed since xunit.v3 went to 4.0.0. ptr727/ProjectTemplate#1088 catalogs the same break across the fleet and recommends the coverlet.MTP route, already round-tripped in ptr727/LanguageTags#325, and this is that migration. - `global.json` opts `dotnet test` into Microsoft.Testing.Platform. The hub cannot supply this file, since it lives in each consumer's own root. - `xunit.runner.visualstudio` is dropped, being the VSTest adapter. - `coverlet.collector` becomes `coverlet.MTP`, whose self-registration codegen needs the package on the test project's compile-time closure, so the `PrivateAssets`/`IncludeAssets` restriction the VSTest data collector carried is not re-applied. - The CI unit-test step becomes `dotnet test --coverlet --coverlet-output-format cobertura --results-directory ./coverage`, which still lands Cobertura XML where the existing Codecov step reads it. - `Docker/Dockerfile` copies `global.json` into the builder, without which `Docker/Build.sh`'s own `dotnet test` falls back to VSTest and fails the image build the same way. The rest of the group's bumps ride along, and `ptr727.LanguageTags` and `ptr727.Utilities` go to their current versions rather than the ones the stale Dependabot PR names. `dotnet outdated` reports nothing outdated, and `.config/dotnet-tools.json` is already at the current csharpier 1.3.0, husky 0.9.1 and dotnet-outdated-tool 4.8.1. Verified locally on the .NET 10.0.400 SDK: 239 of 239 tests pass under MTP, Cobertura XML is produced, and `csharpier check` and `dotnet format style --verify-no-changes` are clean. * Adopt the hub-hosted reusable workflow chain `spec/divergences.json` gives `build-release-task.yml`, `build-docker-task.yml` and `get-version-task.yml` a `retire` disposition, and the audit reported `publish-release.yml` missing its `plan` and `validate` jobs and `merge-bot-pull-request.yml` missing `merge-bot`. Both findings are the same migration, which the hub's `docs/reusable-workflows.md` tracks with PlexCleaner as an open checkbox on stages 1, 3 and 4. - `merge-bot-pull-request.yml` becomes the caller stub. The hub task is a superset of the two jobs carried here, and this repo passes no `rules` or `delete-branch` input: it has no tracker branch outside the built-in pairs, and `delete_branch_on_merge` is off fleet-wide. - `publish-release.yml` gains `plan` (the hub's `publish-plan-task`), which replaces the `ref_name in (main, develop)` job guard and now fails a dispatch from any other branch rather than skipping silently, and `validate`, which is the gate the retired local `build-release-task` used to run inside itself. `publish` calls the hub's `build-release-task`, and `publish-docker-readme` calls the hub's task for the Docker Hub overview, which the retired local Docker task used to push as a step. - `test-pull-request.yml` keeps its `push`-on-every-branch trigger and its aggregator, and its `smoke-build` now calls the hub release task. Both callers move to `permissions: {}` with a per-job `contents: read` grant, in place of the repository default of `write`. - `build-executable-task.yml` is deleted with them. It is not a hub-hosted file, but the hub's `dotnet-publish` default builds the same seven runtimes with the same MSBuild properties and archives the same `PlexCleaner.7z`, so no hook is needed. The `docker-prepare` default likewise emits this repo's exact tags, `./Docker/Dockerfile` context and cache repository. `validate-task.yml` stays carried: the hub's own unit-test step is the VSTest invocation Microsoft.Testing.Platform rejects (ptr727/ProjectTemplate#1088). `WORKFLOW.md` is reconciled against the new job graph, including all three flow diagrams, D0.4 for the pin contract, the D4.1 dispatch-guard change, D4.6 moving the validate gate to the caller, D9.5 naming a carried copy of a hub task a defect, and a new S15 trace for a hub-pin bump. `ARCHITECTURE.md`, `GOVERNANCE.md` "Repository Layout", `OPERATIONS.md` and `PlexCleaner.slnx` follow the same sweep. Verified locally: `actionlint` clean over the four workflows, markdownlint, cspell, ruff, ruff format, mypy and editorconfig-checker clean. * Exclude build output from the EditorConfig check The hub's `.editorconfig-checker.json` excludes the Python cache directories and nothing else, because editorconfig-checker normally scopes itself with `git ls-files` and never sees ignored build output. That fails in a git worktree, which is the layout `repo-worktree` mandates for every task: `.git` is a pointer file there, so `git` is unusable inside the linter's container and it falls back to walking the filesystem. It then reports 25 end-of-line errors against `.artifacts/**` `.pdb`, `.cache`, and the MTP-generated `SelfRegisteredExtensions.cs` after any local build. `.editorconfig-checker.json` is `intent` fidelity in `spec/files.json`, so this repo adds the three patterns its own `.gitignore` already lists. Reproduced before the change and clean after, in this worktree. * Select the test project with --project in the Docker build The Docker image build failed on this branch with "Specifying a project for 'dotnet test' should be via '--project'". Under Microsoft.Testing.Platform, which `global.json` now opts into, no 10.x SDK honors a positional project path, and the two SDKs this repo meets differ only in how loudly they say so. The 10.0.400 SDK the host pins ignores the path and resolves from the current directory, which is why this passed locally. The newer `dotnet-sdk-10.0` apt package the builder stage installs from `ubuntu:rolling` fails outright. The quiet arm is the worse one in the builder, since `/Builder` carries neither `PlexCleaner.slnx` nor a root project for that fallback to find, so `--project` is required there rather than merely tidier. Verified by rebuilding the builder stage with `--no-cache-filter builder`: 239 of 239 tests pass inside the container. * Answer the first review round Three findings from the review, each verified before acting on it. `createschema` now writes `PlexCleaner.schema.json` with LF and a trailing newline, so a regeneration is byte-identical to the tracked file instead of a whole-file working-tree diff against the repo's new LF default. The line ending is set on that one write rather than on `ConfigFileJsonContext`, which was the first attempt. That context also writes the user's own `PlexCleaner.json`, through `WriteDefaultsToFile`, `OpenAndUpgrade`'s schema upgrade, and the `RegisterInvalidFiles` ignore-list update, so flipping it there would silently rewrite a Windows user's settings file to LF on the next run that upgrades it. `PlexCleaner.defaults.json` is not a beneficiary either way: its comments are hand-restored and its `UseSystem`/`AutoUpdate` values are platform-dependent, so it cannot be regenerated in place at all. The sidecar, tool-info and process-result contexts keep CRLF, unchanged. Verified: `createschema` leaves the tree clean, and `defaultsettings` against a scratch path still writes CRLF. `publish-release.yml`'s comment above the `plan` job said a feature-branch dispatch resolves `publish == false`. The hub task exits with an `::error::` instead, which this branch's own D4.1 already states. `RegressionTests/README.md` said Python source is CRLF, which this branch made untrue. * Deserialize idet theory data to the type xUnit asked for `FfMpegIdetInfoSerializer.Deserialize` called `JsonSerializer.Deserialize<FfMpegIdetInfoSerializer>`, deserializing into the serializer class rather than into `FfMpegIdetInfo`, and fell back to a bare `new object()` on null. Both arms return something the `Parse_Idet_Parse_Test` theory's `FfMpegIdetInfo` parameter cannot accept, so any execution path that round-trips theory data through the registered serializer fails at the parameter bind rather than in the serializer. Proved by reflection against the built test assembly: with the old code `Deserialize(typeof(FfMpegIdetInfo), json)` returns `PlexCleanerTests.FfMpegIdetInfoSerializer`, and with the new code it returns `PlexCleaner.FfMpegIdetInfo`. The whole suite is unaffected either way, 239 of 239 before and after, because the in-process run hands theory data to the test directly rather than through the serializer, which is why this has sat latent. Pre-existing rather than introduced here, but the file is in the test project this branch migrates to Microsoft.Testing.Platform, the execution host that decides whether the round-trip happens at all, and the fix is one line. Raised by Copilot as a body-only finding on all three review rounds.
Resolves one modify/delete conflict on `.github/workflows/build-docker-task.yml`, taking `develop`'s deletion. `main` had bumped `docker/setup-buildx-action` to v4.3.0 inside that file (#934), while `develop` retired it: the file is hub-hosted per `spec/divergences.json`, and the publisher now reaches `ptr727/ProjectTemplate/.github/workflows/build-docker-task.yml` by pin instead. Nothing is lost, because the hub task at the pinned commit already carries the identical SHAs for every action `main` had bumped locally: `setup-buildx-action` v4.3.0 `37fe631`, `login-action` v4.6.0 `dbcb813`, and `checkout` v7.0.1 `3d3c42e`. The resulting tree is byte-identical to `develop`, so `main` carried no other unique content. Verified on the merge result: build clean with `TreatWarningsAsErrors`, 239 of 239 tests pass.
|
Warning Review limit reachedNext included review available in 51 seconds. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (213)
Comment |
PR Summary by QodoPromote Hub Resync and Centralize Release Workflows
AI Description
Diagram
High-Level Assessment
Files changed (204)
|
Code Review by Qodo
1. PR title breaks title case
|
Code Review by Qodo
1. PR title breaks title case
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔵 Needs a closer look
It is a broad promotion touching CI/workflow wiring, build/release infrastructure, and test/coverage runner behavior across many files.
Pull request overview
Promotes the already-merged hub resync work from develop to main, resolving the single modify/delete conflict by accepting develop’s retirement of the local Docker build task and standardizing the repo on the hub-hosted reusable workflow chain and Microsoft.Testing.Platform-based test/coverage flow.
Changes:
- Retires locally-carried config/spec/workflow task files in favor of hub-hosted reusable workflows and hub-side configuration sources.
- Switches unit test + coverage execution to Microsoft.Testing.Platform with
coverlet.MTPand updates CI + Docker build to match. - Normalizes repo text handling (notably line endings via
.gitattributes) and updates supporting docs/tooling files accordingly.
File summaries
| File | Description |
|---|---|
| version.json | No functional change; formatting/normalization only. |
| spec/secrets.json | Removed (retired local secrets spec). |
| Sandbox/TestSomething.cs | Formatting/normalization only. |
| Sandbox/Sandbox.csproj | Formatting/normalization only. |
| Samples/PlexCleaner/PlexCleaner.v1.json | Formatting/normalization only. |
| repo-config/settings.json | Removed (retired repo-config payload). |
| repo-config/main.json | Removed (retired repo-config payload). |
| repo-config/develop.json | Removed (retired repo-config payload). |
| RegressionTests/synthetic/inject_cc_sei.py | Formatting/normalization only. |
| RegressionTests/reduction-rules.example.json | Formatting/normalization only. |
| RegressionTests/pyproject.toml | Formatting/normalization only. |
| Plugins/MatroskaHeaderCleanup/MatroskaHeaderCleanup.csproj | Formatting/normalization only. |
| Plugins/DtsTimestampRepair/DtsTimestampRepair.csproj | Formatting/normalization only. |
| PlexCleanerTests/WildcardTests.cs | Formatting/normalization only. |
| PlexCleanerTests/ToolProgressParsingTests.cs | Formatting/normalization only. |
| PlexCleanerTests/StreamTimingsProbeTests.cs | Formatting/normalization only. |
| PlexCleanerTests/ProcessDriverTests.cs | Formatting/normalization only. |
| PlexCleanerTests/PluginLoaderTests.cs | Formatting/normalization only. |
| PlexCleanerTests/PlexCleanerTests.csproj | Migrate test coverage package to coverlet.MTP; drop VSTest runner adapter. |
| PlexCleanerTests/FfProbeLogTests.cs | Formatting/normalization only. |
| PlexCleanerTests/FfMpegNalUnitTests.cs | Formatting/normalization only. |
| PlexCleanerTests/FfMpegIdetInfoSerializer.cs | Fix serializer deserialization target type and null handling. |
| PlexCleanerTests/ClosedCaptionsProbeTests.cs | Formatting/normalization only. |
| PlexCleanerTests/.editorconfig | Formatting/normalization only. |
| PlexCleaner/VerifyResult.cs | Formatting/normalization only. |
| PlexCleaner/VerifyOptions.cs | Formatting/normalization only. |
| PlexCleaner/VerifyClassifier.cs | Formatting/normalization only. |
| PlexCleaner/ToolsOptions.cs | Formatting/normalization only. |
| PlexCleaner/ToolInfoJsonSchema.cs | Formatting/normalization only. |
| PlexCleaner/TagMap.cs | Formatting/normalization only. |
| PlexCleaner/PerFileLogLevel.cs | Formatting/normalization only. |
| PlexCleaner/MkvProcess.cs | Formatting/normalization only. |
| PlexCleaner/MediaToolInfo.cs | Formatting/normalization only. |
| PlexCleaner/MediaInfoBuilder.cs | Formatting/normalization only. |
| PlexCleaner/Language.cs | Formatting/normalization only. |
| PlexCleaner/KeepAwake.cs | Formatting/normalization only. |
| PlexCleaner/JsonSerialization.cs | Formatting/normalization only. |
| PlexCleaner/IProcessPlugin.cs | Formatting/normalization only. |
| PlexCleaner/GlobalUsings.cs | Formatting/normalization only. |
| PlexCleaner/GitHubRelease.cs | Formatting/normalization only. |
| PlexCleaner/Extensions.cs | Formatting/normalization only. |
| PlexCleaner/DtsInfo.cs | Formatting/normalization only. |
| PlexCleaner/AudioProps.cs | Formatting/normalization only. |
| PlexCleaner/AssemblyVersion.cs | Formatting/normalization only. |
| PlexCleaner.code-workspace | Formatting/normalization only. |
| LICENSE | Formatting/normalization only. |
| host-tools.json | Formatting/normalization only. |
| global.json | Add Microsoft.Testing.Platform runner selection for dotnet test. |
| Docs/LanguageMatching.md | Formatting/normalization only. |
| Docker/README.md | Formatting/normalization only. |
| Docker/Dockerfile | Copy global.json into builder stage so dotnet test uses MTP during image build. |
| Docker/Build.sh | Use dotnet test --project ... to satisfy MTP runner argument requirements. |
| Directory.Packages.props | Update package set for MTP + coverlet.MTP + xUnit updates; remove VSTest-only packages. |
| Directory.Build.props | Formatting/normalization only. |
| codecov.yml | Formatting/normalization only. |
| CLAUDE.md | Add Claude Code entry-point file routing to AGENTS.md. |
| .markdownlint-cli2.jsonc | Formatting/normalization only. |
| .husky/task-runner.json | Formatting/normalization only. |
| .gitignore | Formatting/normalization only. |
| .github/workflows/validate-task.yml | Switch CI test+coverage to coverlet.MTP (--coverlet flow) under MTP. |
| .github/workflows/test-pull-request.yml | CI now calls hub build-release-task.yml for smoke builds; restrict token scopes. |
| .github/workflows/publish-release.yml | Publisher now uses hub reusable release chain + plan/validate gating; restrict token scopes. |
| .github/workflows/merge-bot-pull-request.yml | Replace in-repo merge-bot implementation with hub reusable merge-bot task. |
| .github/workflows/get-version-task.yml | Removed (superseded by hub reusable workflow chain). |
| .github/workflows/build-executable-task.yml | Removed (superseded by hub reusable workflow chain). |
| .github/workflows/build-docker-task.yml | Removed (superseded by hub reusable workflow chain). |
| .github/skills/workflow-ci-contract/references/test-methodology.md | Add workflow testing methodology reference doc. |
| .github/skills/upstream-contribution-workflow/SKILL.md | Add upstream contribution workflow skill doc. |
| .github/skills/shell-codestyle/SKILL.md | Add shell codestyle skill doc. |
| .github/skills/python-codestyle/references/testing.md | Add Python testing conventions reference. |
| .github/skills/python-codestyle/references/profiles.md | Add Python profile details reference. |
| .github/skills/python-codestyle/references/code-style.md | Add Python code style reference. |
| .github/skills/git-commit-conventions/references/history-rewrite.md | Add history rewrite/signing reference. |
| .github/skills/fleet-conformance-check/SKILL.md | Add fleet conformance check skill doc. |
| .github/skills/dotnet-codestyle/references/testing.md | Add .NET testing conventions reference. |
| .github/skills/dotnet-codestyle/references/project-config.md | Add .NET project configuration reference. |
| .github/skills/dotnet-codestyle/references/conventions.md | Add .NET coding standards reference. |
| .github/skills/copilot-instructions-keeper/SKILL.md | Add Copilot instructions keeper skill doc. |
| .github/skills/comment-and-doc-style/references/markdown-links.md | Add reference-style link conventions doc. |
| .github/skills/comment-and-doc-style/references/carried-doc-references.md | Add carried-doc reference rules doc. |
| .github/skills/code-review/SKILL.md | Add code-review skill doc. |
| .github/skills/carried-instruction-file-guard/SKILL.md | Add carried instruction file guard skill doc. |
| .github/skills/audit-a-repo/SKILL.md | Add audit-a-repo skill doc. |
| .github/skills/add-host-tool/SKILL.md | Add add-host-tool skill doc. |
| .github/ISSUE_TEMPLATE/config.yml | Formatting/normalization only. |
| .github/dependabot.yml | Formatting/normalization only. |
| .gitattributes | Normalize text files to LF by default; pin .bat/.cmd to CRLF. |
| .editorconfig-checker.json | Add excludes for common generated/cache directories. |
| .dockerignore | Formatting/normalization only. |
| .config/dotnet-tools.json | Formatting/normalization only. |
Review details
- Files reviewed: 73/213 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ptr727
changed the title
Owner
Author
|
Answering both Qodo findings here as well as in their threads, since a Qodo finding carries no resolvable state of its own. 1. "PR title breaks title case" (📘 Rule violation, ✧ Quality). Correct, and fixed: the title is now "Promote Develop to Main for the Hub Resync". GOVERNANCE.md "Pull Request Title and Commit Message Conventions" is title case with lowercase short bind words (a, an, the, and, but, or, of, in, on, at, to, by, for, from), so 2. "Publishing triggers are misstated" (🐞 Bug, ≡ Correctness). The observation is right and the location is not, so declining rather than editing. Verified against the diff rather than taking the badge: this repository's The file it flags, What governs this repository is its own If the fleet wants that variant enumerated in the skill, it is a ptr727/ProjectTemplate change that re-vendors everywhere, not a PlexCleaner one. |
…ge (#943) * Switch the coverage engine to Microsoft.Testing.Extensions.CodeCoverage ptr727/ProjectTemplate#1088 resolved its open decision to `Microsoft.Testing.Extensions.CodeCoverage` rather than `coverlet.MTP`, and hub `WORKFLOW.md` D1.6 now binds an MTP-based repo, in practice any repo on xunit.v3 4.0.0 or later, to that package at 18.9.0 or later in place of `coverlet.collector`. PlexCleaner became MTP-based in #940, so the "migration still owed rather than drift" exemption D1.6 grants a repo that is not yet MTP-based no longer covers it. #940 chose coverlet.MTP on the strength of the issue's original recommendation, before that decision was recorded. - `Microsoft.Testing.Extensions.CodeCoverage` 18.10.0 replaces `coverlet.MTP`, central-managed as an exact pin rather than a range. - The CI step becomes `dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, byte-identical to the hub validator's own, so the two gates cannot drift on coverage. - `--coverage-output` stays unset, since pinning one filename gives every test project in the solution the same path and the last to finish overwrites the rest. The `<guid>.cobertura.xml` default it writes instead is a name `codecov-cli`'s file finder does not match, its patterns being `*coverage*.*` and an exact `cobertura.xml`, so each report is prefixed to `coverage-<guid>.cobertura.xml` rather than renamed, keeping the guid. - `.gitignore` gains `*.cobertura.xml` and `TestResults/`, which D1.6 names. A run that passes no results directory writes `./TestResults/<guid>.cobertura.xml`. The 18.9.0 floor takes two reasons and the comments now carry both, because the first alone would justify lowering it to 18.1.0. Below 18.1.0 the extension is built against Microsoft.Testing.Platform 1.x and an 18.0.x resolution throws a `TypeLoadException` against the 2.x platform xunit.v3 4.0.0 carries, runs zero tests, and still writes a well-formed Cobertura file reporting full coverage, so only the non-zero exit says the run reported nothing. 18.9.0 is then the first release on Microsoft.Testing.Platform 2.3.x, where every test project writes into the one shared results directory the invocation names. Verified on the .NET 10.0.400 SDK: the graph resolves 18.10.0 over Microsoft.Testing.Platform 2.3.3, 239 of 239 tests pass at exit 0, the report carries real data (`line-rate="0.501"`) rather than the full-coverage artifact the floor warns about, the raw name is `<guid>.cobertura.xml` and the prefixed name matches codecov's finder, and the Docker builder's own `dotnet test` still passes 239 of 239. Also corrects four sites, and one comment this change added, that said the hub still ran the VSTest invocation and that adopting its task would be a deletion. ptr727/ProjectTemplate#1107 migrated the hub's step, so that is no longer the blocker. What blocks adoption now is that the hub's task declares no `ref` input and checks out the caller's default ref, where `publish-release.yml`'s `validate` job passes `github.sha` so the publish gate validates the exact commit being published, per D4.6. * Name the package this change actually replaces Both comments described the extension as replacing `coverlet.collector`, which is D1.2's own wording for the fleet baseline but not what happens here: this repository was on `coverlet.MTP`, chosen in #940 before ptr727/ProjectTemplate#1088 resolved the fleet on this package. A reader of these files sees a replacement that did not occur. Both now name `coverlet.MTP` as the thing displaced and keep `coverlet.collector` as the baseline further back, so the comment matches the diff and still connects to D1.2's wording. The test project's comment also drops the sentence about `xunit.runner.visualstudio`, which #940 removed and this change does not touch, and records instead that the compile-time closure requirement is enforced by a build failure, since the generated `SelfRegisteredExtensions.cs` calls into the package directly.
There was a problem hiding this comment.
🟢 Approval recommended
The changes look consistent with the stated promotion/resync intent, and the only identified issue is a minor test assertion-style inconsistency.
Review details
- Files reviewed: 73/213 changed files
- Comments generated: 0 new
- Review effort level: Lite
Owner
Author
|
Round 2 on head Copilot: 🟢 Approval recommended. Its one remaining note is "a minor test assertion-style inconsistency", which is the native-xUnit-assertions item already inventoried and deferred as #941. That is unchanged by this promotion, and all four affected files are line-endings-only in this diff. Qodo's "PR title breaks title case" is stale. It was raised on round 1 and fixed then; the title has read "Promote Develop to Main for the Hub Resync" since, which satisfies GOVERNANCE.md's title case with lowercase short bind words. Qodo carries no resolvable state of its own, so it re-lists the finding rather than clearing it. Its second round-1 finding, on the carried What changed since round 1. State: |
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.
Why
Promotes the hub resync in #940 (merged to
developas0e9b0b9) tomain.The head is
promote/develop-to-main-2026-08-30rather thandevelopitself, because the promotion carries one conflict anddevelop'srequired_linear_historyplus its PR ruleset forbid resolving it there. That is the procedure the fleet'soperational-vs-release-workflowreference sets for a conflicted promotion: resolve on a throwaway branch offmainand open that intomain.developis untouched and is not deleted.The one conflict, and why the resolution loses nothing
.github/workflows/build-docker-task.yml, modify/delete.developretired it (spec/divergences.jsongives it aretiredisposition, and the publisher now reaches the hub's copy by SHA pin), whilemainhad bumpeddocker/setup-buildx-actioninside it to v4.3.0 in #934. Resolved by takingdevelop's deletion.Nothing is lost, because the hub's
build-docker-task.ymlat the pinned commita3781210c622a61bdd3bbe31a6ac7a431c4a163dalready carries byte-identical SHAs for every actionmainhad bumped in the local copy:main's local pindocker/setup-buildx-action37fe631v4.3.037fe631v4.3.0docker/login-actiondbcb818v4.6.0dbcb818v4.6.0actions/checkout3d3c42ev7.0.13d3c42ev7.0.1The Docker Hub overview push that the deleted file did as an in-job step is relocated rather than dropped: it is now the
publish-docker-readmejob reaching the hub'spublish-docker-readme-task.yml, same action, same repository, sameDocker/README.md, still gated tomain.What
maincarried thatdevelopdid notMeasured against the true merge-base
57d81f5(the last promotion, #924),main's entire unique delta is three action-pin bumps across two files:main-only commitc621049build-docker-task.yml:setup-buildx-actionv4.2.0 -> v4.3.056f9268validate-task.yml:cspell-actionv8.4.0 -> v9.0.1validate-task.yml:82b75d4d6validate-task.yml:setup-uvv9.0.0 -> v10.0.1validate-task.yml:94Everything else in
git log origin/develop..origin/mainis a merge commit or a bump whose content was already at the merge-base.Directory.Packages.props, the usual silent-loss candidate, is unchanged onmainsince the base whiledevelopmoved every package forward.The merge tree is therefore byte-identical to
develop(treea2e56c5on both). That is the correct outcome here rather than evidence of dropped work:mainhad no unique content left once the two surviving bumps landed ondevelopindependently at the same SHAs and the third file was retired.Verification
main's. Every SHA-pinneduses:matchesmainexactly, and no NuGet package is downgraded.TreatWarningsAsErrors, 239 of 239 tests pass on the merge result.