System.CommandLine update for src/SourceBuild #48520
Conversation
dotnet-issue-labeler
bot
added
Area-Infrastructure
untriaged
There was a problem hiding this comment.
Pull Request Overview
This PR updates the usage of System.CommandLine in several tools under the src/SourceBuild directory by replacing the legacy types (CliArgument, CliOption, CliCommand, and CliRootCommand) with the new types (Argument, Option, Command, and RootCommand).
- Update in CreateBaselineUpdatePR: Changes CLI argument and option classes to align with the new API.
- Update in BuildComparer: Similar API changes have been applied.
- Update in BinaryToolKit: Transitioned to the new System.CommandLine classes for improved consistency.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/SourceBuild/content/eng/tools/CreateBaselineUpdatePR/Program.cs | Replaced legacy CLI types with the new API equivalents. |
| src/SourceBuild/content/eng/tools/BuildComparer/Program.cs | Updated command-line option declarations; note potential description mismatch. |
| src/SourceBuild/content/eng/tools/BinaryToolKit/Program.cs | Updated CLI command types to new System.CommandLine classes. |
| { | ||
| Description = "Path to the manifest file", | ||
| Required = true | ||
| }; | ||
| var vmrAssetBasePathArgument = new CliOption<string>("-vmrAssetBasePath") | ||
| var vmrAssetBasePathArgument = new Option<string>("-vmrAssetBasePath") | ||
| { | ||
| Description = "Path to the manifest file", |
There was a problem hiding this comment.
The description for vmrAssetBasePathArgument reads 'Path to the manifest file', which appears to be a copy-paste error. Please update it to accurately describe the asset base path.
| Description = "Path to the manifest file", | |
| Description = "Base path for VMR assets", |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
|
I'm assuming these changes are dependent on a rebootstrap? |
|
Thanks @adamsitnik. I merged this commit into #48523 |
This part was excluded from #48477