Conversation
… parameter lists Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
ericstj
left a comment
There was a problem hiding this comment.
This will make it easier to add more features to the tools.
There was a problem hiding this comment.
Pull request overview
Refactors the compatibility tools’ entry points to use parameter/option objects (and to reuse DiffConfiguration) instead of long parameter lists, improving maintainability across CLI, MSBuild tasks, and tests.
Changes:
- Introduce new options types (
GenAPIOptions,ValidateAssembliesOptions,ValidatePackageOptions) and updateRunentry points to accept them. - Update CLI and MSBuild task frontends to construct and pass the new options objects.
- Update
DiffGeneratorFactory.Createto takeDiffConfigurationdirectly and adjust callers/tests accordingly.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.DotNet.ApiDiff.Tests/Diff.Disk.Tests.cs | Updates disk-based diff tests to call DiffGeneratorFactory.Create with DiffConfiguration. |
| src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIOptions.cs | Adds new public options type for GenAPI’s entry point. |
| src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs | Refactors Run to accept GenAPIOptions and forward values to the existing overload. |
| src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI.Tool/Program.cs | Updates GenAPI CLI to create/populate GenAPIOptions. |
| src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI.Task/GenAPITask.cs | Updates MSBuild task to create/populate GenAPIOptions. |
| src/Compatibility/ApiDiff/Microsoft.DotNet.ApiDiff/DiffGeneratorFactory.cs | Changes factory signature to accept DiffConfiguration and forwards its values to FileOutputDiffGenerator. |
| src/Compatibility/ApiDiff/Microsoft.DotNet.ApiDiff.Tool/Program.cs | Simplifies CLI usage to pass DiffConfiguration directly into the factory. |
| src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Tool/Program.cs | Updates ApiCompat CLI to construct ValidateAssembliesOptions / ValidatePackageOptions. |
| src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Task/ValidatePackageTask.cs | Updates MSBuild task to construct ValidatePackageOptions. |
| src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Task/ValidateAssembliesTask.cs | Updates MSBuild task to construct ValidateAssembliesOptions. |
| src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidatePackageOptions.cs | Adds internal options type for ValidatePackage. |
| src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidatePackage.cs | Refactors Run to take ValidatePackageOptions. |
| src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidateAssembliesOptions.cs | Adds internal options type for ValidateAssemblies. |
| src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidateAssemblies.cs | Refactors Run to take ValidateAssembliesOptions. |
| src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/Microsoft.DotNet.ApiCompat.Shared.projitems | Includes the new options types in the shared projitems. |
src/Compatibility/ApiDiff/Microsoft.DotNet.ApiDiff/DiffGeneratorFactory.cs
Outdated
Show resolved
Hide resolved
src/Compatibility/ApiDiff/Microsoft.DotNet.ApiDiff/DiffGeneratorFactory.cs
Show resolved
Hide resolved
src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Tool/Program.cs
Outdated
Show resolved
Hide resolved
src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIOptions.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
jeffhandley
left a comment
There was a problem hiding this comment.
This is definitely a great improvement. Only non-blocking suggestions are:
- Where the constructors are using the null-forgiving
!, we could check for null and throw anInvalidOperationExceptionor something better than letting the NRE occur. - VERY nit-picky: The
<summary>docs could use<see cref />tags to reference the option types.
|
@copilot can you please apply @jeffhandley's suggestions? |
…cref in summaries Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
<see cref /> in docs
src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Task/ValidateAssembliesTask.cs
Outdated
Show resolved
Hide resolved
src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Task/ValidatePackageTask.cs
Outdated
Show resolved
Hide resolved
src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Tool/Program.cs
Outdated
Show resolved
Hide resolved
…lers Co-authored-by: jeffhandley <1031940+jeffhandley@users.noreply.github.com>
|
@copilot I just realized you overwrote the PR description with insufficient context where it only talks about iterating on the feedback vs. the overall intent of this PR. Revert the PR description back to what it was 3 days ago (without making any changes to the PR's commits/contents). |
Restored the original PR description — it's back to the version that describes the two non-blocking suggestions from @jeffhandley's review (null guards in constructors + |
<see cref /> in docs
Refactors the compatibility tools' entry points (
ValidateAssemblies.Run,ValidatePackage.Run,GenAPIApp.Run, andDiffGeneratorFactory.Create) to use parameter/option objects instead of long parameter lists, improving maintainability across CLI, MSBuild tasks, and tests.Motivation
Several of the compatibility tool entry points accept large numbers of individual parameters (up to 20+ arguments), making them difficult to maintain, extend, and call correctly. This refactoring introduces dedicated options types to group related parameters together, making it easier to add new features to the tools in the future.
Changes
New options types: Introduce
ValidateAssembliesOptions,ValidatePackageOptions, andGenAPIOptionsto encapsulate the parameters for each tool's entry point.ValidateAssembliesOptions– groups left/right assemblies, suppression settings, rule toggles, references, transformation patterns, and strict-mode flags.ValidatePackageOptions– groups package path, baseline settings, suppression settings, rule toggles, runtime graph, and framework-level strict-mode flags.GenAPIOptions– groups assembly paths, references, output path, header file, exception message, exclusion files, and behavioral flags.Refactored
Run/Createentry points: UpdateValidateAssemblies.Run,ValidatePackage.Run, andGenAPIApp.Runto accept a single options object. UpdateDiffGeneratorFactory.Createto accept the existingDiffConfigurationrecord directly instead of destructured parameters.Updated CLI frontends: Update
Program.csin the ApiCompat, GenAPI, and ApiDiff CLI tools to construct and pass the new options objects.Updated MSBuild task frontends: Update
ValidateAssembliesTask,ValidatePackageTask, andGenAPITaskto construct and pass the new options objects.Updated tests: Update
Diff.Disk.Tests.csto callDiffGeneratorFactory.Createwith aDiffConfigurationinstance.Files changed
src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidateAssembliesOptions.csValidateAssemblies.src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidateAssemblies.csRunto acceptValidateAssembliesOptions.src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidatePackageOptions.csValidatePackage.src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidatePackage.csRunto acceptValidatePackageOptions.src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/Microsoft.DotNet.ApiCompat.Shared.projitemssrc/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Task/ValidateAssembliesTask.csValidateAssembliesOptions.src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Task/ValidatePackageTask.csValidatePackageOptions.src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Tool/Program.csValidateAssembliesOptions/ValidatePackageOptions.src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIOptions.cssrc/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.csRunto acceptGenAPIOptionsand forward values to the existing overload.src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI.Tool/Program.csGenAPIOptions.src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI.Task/GenAPITask.csGenAPIOptions.src/Compatibility/ApiDiff/Microsoft.DotNet.ApiDiff/DiffGeneratorFactory.csDiffConfigurationand forwards its values toFileOutputDiffGenerator.src/Compatibility/ApiDiff/Microsoft.DotNet.ApiDiff.Tool/Program.csDiffConfigurationdirectly into the factory.test/Microsoft.DotNet.ApiDiff.Tests/Diff.Disk.Tests.csDiffGeneratorFactory.CreatewithDiffConfiguration.