[devops] Vendor Maestro changelog tool - #26461
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR vendors the Maestro changelog generator into the repo under scripts/changelog and updates the maestro-changelog GitHub Actions workflow to run the local tool instead of cloning spouliot/dotnet-tools.
Changes:
- Add a new
scripts/changelog.NET console tool (README, csproj, and Makefile fragment). - Update
.github/workflows/maestro-changelog.ymltocheckoutthe repo and run the vendored tool viadotnet run. - Integrate the tool with the repo’s script templating mechanism via
fragment.mk.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/changelog/README.md | Documents the new vendored changelog tool and its invocation. |
| scripts/changelog/Program.cs | Implements the changelog generator logic (diff parsing + nested dependency traversal). |
| scripts/changelog/fragment.mk | Adds Makefile template integration for the script. |
| scripts/changelog/changelog.csproj | Defines the tool project targeting net$(BundledNETCoreAppTargetFrameworkVersion). |
| .github/workflows/maestro-changelog.yml | Switches workflow to use the vendored tool instead of cloning an external repo. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Suppressed comments (1)
.github/workflows/maestro-changelog.yml:26
⚠️ This workflow installs .NET 9 explicitly, but the vendored tool targetsnet$(BundledNETCoreAppTargetFrameworkVersion). In this repoglobal.jsonpins a .NET 10 preview SDK, so this job is likely to fail to build/run the tool. Useglobal-json-file: ./global.json(as other workflows do) so the action uses the repo-pinned SDK.
- uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0
with:
dotnet-version: '9'
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
scripts/changelog/Program.cs:14
- 🤖 💡 suggestion Code organization — These lists are initialized once and never reassigned; marking them
readonlyprevents accidental reassignment and makes the intent clearer.
Rule: Prefer immutable fields when possible
static List<string> list = new ();
static List<string> filters = new ();
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
scripts/changelog/Program.cs:34
- 🤖
⚠️ Documentation — The generated header uses inconsistent casing (".net ChangeLog"), which reads as a typo in the posted PR comment. Use the standard ".NET" casing and consistent "Changelog" capitalization.
writer.WriteLine ($"# .net ChangeLog for {pr}");
scripts/changelog/Program.cs:37
- 🤖
⚠️ Bug — The GitHub URL rewrite unconditionally appends ".diff". If a user passes a GitHub diff URL (already ending in ".diff") or any non-PR GitHub URL, this will produce an invalid URL (for example ".diff.diff"). Restrict the rewrite to PR page URLs and avoid double-appending.
if (pr.StartsWith ("https://github.com/", StringComparison.Ordinal)) {
pr = pr.Replace ("https://github.com/", "https://patch-diff.githubusercontent.com/raw/") + ".diff";
}
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
scripts/changelog/changelog.csproj:9
- 💡
scripts/Directory.Build.propsalready sets<OutputType>Exe</OutputType>for all script projects, so this project-level setting is redundant and makes the script csproj differ from the other scripts underscripts/. Consider removing it and relying on the shared scripts props.
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)</TargetFramework>
</PropertyGroup>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
scripts/changelog/Program.cs:120
- Removed dependencies (only "-" with no matching "+") aren’t reported today. Also, the current
returnwhen encountering the nextdiff --githeader prevents any end-of-file flushing logic from running. Switching thereturnto abreakmakes it possible to flush a pending removed dependency after the loop, and adding a flush when a new<Uri>is encountered ensures intermediate removals are captured too.
This issue also appears on line 137 of the same file.
if (line.StartsWith ("diff --git ", StringComparison.Ordinal))
return;
if (line.Length < 1)
continue;
bool removal = (line [0] == '-');
scripts/changelog/Program.cs:10
- Namespace names in this repo’s script tools are typically PascalCase (e.g.
Xamarin.Bundler,GenerateFrameworksConstants). Using a lowercasenamespace changelogis inconsistent and makes the code look like a package name instead of a .NET namespace; consider renaming it toChangelog(or similar).
namespace changelog {
scripts/changelog/Program.cs:138
- After breaking out of the loop when the Version.Details.xml diff ends, a pending removed dependency (old SHA set, new SHA empty) should be flushed; otherwise removals at the end of the file are still omitted from the changelog output.
}
}
This comment has been minimized.
This comment has been minimized.
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #631aea4] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 1 tests failed, 204 tests passed. Failures❌ windows tests1 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Replace the external dotnet-tools clone in the Maestro changelog workflow with a vendored scripts/changelog project.
The workflow now checks out the repository and runs the local tool instead of cloning spouliot/dotnet-tools.
🤖 Pull request created by Copilot