Improve .NET SDK build infrastructure and documentation#643
Improve .NET SDK build infrastructure and documentation#643stephentoub wants to merge 1 commit intogithub:mainfrom
Conversation
stephentoub
commented
Mar 3, 2026
- Add Central Package Management (Directory.Packages.props) with all package versions centralized
- Add Directory.Build.props with shared properties (TargetFramework, ImplicitUsings, Nullable, TreatWarningsAsErrors)
- Add nuget.config with single nuget.org source and package source mapping (required by CPM)
- Add global.json specifying .NET 10 SDK (the library is still built with a net8.0 TFM)
- Update all CI workflows from .NET 8.0.x to .NET 10.0.x
- Enable XML documentation file generation (GenerateDocumentationFile) so that comments are validated and IntelliSense docs are generated
- Add XML doc comments to all non-generated public types and members
- Add valid-value lists in XML docs for string properties with known values (e.g. PermissionRequest.Kind, ToolResultObject.ResultType)
- Add #pragma warning disable CS1591 to generated files (SessionEvents.cs, Rpc.cs) and codegen scripts
- Enable EmbedUntrackedSources, IncludeSymbols, SymbolPackageFormat
- Enable ContinuousIntegrationBuild conditional on CI/TF_BUILD environment variables
- Add PackageProjectUrl to package metadata
- Add [EditorBrowsable(Never)] to obsolete GithubToken property
- Upgrade analysis level and fix some diagnostics
There was a problem hiding this comment.
Pull request overview
This PR modernizes the .NET SDK’s build/tooling setup (centralized package/version management, updated SDK tooling in CI) and improves public API documentation by enabling XML doc generation and adding XML docs across the SDK (with CS1591 suppression for generated code).
Changes:
- Introduce Central Package Management + shared build props (TFM/lang/analyzers/warnings-as-errors) and add NuGet source mapping + dotnet/global.json.
- Update CI workflows to use .NET 10 SDK and adjust codegen to suppress CS1591 in generated C# files.
- Add/expand XML documentation across public .NET SDK types and tighten some implementation details (logging, parsing, small refactors).
Reviewed changes
Copilot reviewed 25 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/codegen/csharp.ts | Suppress CS1591 in generated C# and tweak default initializers for generated types. |
| nodejs/scripts/update-protocol-version.ts | Adjust generated C# protocol version constant visibility. |
| dotnet/test/ToolsTests.cs | Test refactors and modern C# syntax changes (incl. permission handler). |
| dotnet/test/SessionTests.cs | Use collection expressions for AvailableTools/ExcludedTools in tests. |
| dotnet/test/Harness/E2ETestContext.cs | Seal type + expand expression-bodied members for style/analyzer compliance. |
| dotnet/test/Harness/E2ETestBase.cs | Expand expression-bodied members and update collection initialization style. |
| dotnet/test/Harness/CapiProxy.cs | Seal type + collection expression usage and expanded DisposeAsync. |
| dotnet/test/GitHub.Copilot.SDK.Test.csproj | Move package versions to CPM (remove Version=...). |
| dotnet/test/ClientTests.cs | Minor test cleanup (collection expressions, ThrowsAsync simplification). |
| dotnet/src/Types.cs | Add extensive XML docs and modernize initializers; adjust API metadata attributes. |
| dotnet/src/Session.cs | Seal session class + refactors (handlers, initializers, formatting). |
| dotnet/src/SdkProtocolVersion.cs | Hide constant and expose accessor; formatting changes. |
| dotnet/src/GitHub.Copilot.SDK.csproj | Enable XML docs + add pack/sourcelink/CI build metadata; move to CPM. |
| dotnet/src/Generated/SessionEvents.cs | Suppress CS1591 and small formatting changes. |
| dotnet/src/Generated/Rpc.cs | Suppress CS1591 and update empty collection initialization style. |
| dotnet/src/Client.cs | Seal client class + logging/formatting/culture tweaks; regex source-gen; minor refactors. |
| dotnet/samples/Chat.csproj | Remove duplicated per-project props now covered by Directory.Build.props. |
| dotnet/nuget.config | Add single source + package source mapping for CPM. |
| dotnet/global.json | Pin .NET SDK selection for dotnet/ folder builds. |
| dotnet/Directory.Packages.props | Centralize NuGet package versions for dotnet projects. |
| dotnet/Directory.Build.props | Centralize shared build settings (TFM, LangVersion, analyzers, warnings-as-errors). |
| .github/workflows/update-copilot-dependency.yml | Update CI to install .NET 10. |
| .github/workflows/scenario-builds.yml | Update CI to install .NET 10. |
| .github/workflows/publish.yml | Update CI to install .NET 10. |
| .github/workflows/dotnet-sdk-tests.yml | Update CI to install .NET 10. |
| .github/workflows/docs-validation.yml | Update CI to install .NET 10. |
| .github/workflows/copilot-setup-steps.yml | Update CI to install .NET 10. |
| /// </code> | ||
| /// </example> | ||
| public partial class CopilotClient : IDisposable, IAsyncDisposable | ||
| public sealed partial class CopilotClient : IDisposable, IAsyncDisposable |
There was a problem hiding this comment.
CopilotClient being changed to sealed is a breaking public API change (consumers can no longer derive from it). If this is unintentional, revert the sealed modifier; if intentional, consider documenting the breaking change and versioning accordingly.
| public sealed partial class CopilotClient : IDisposable, IAsyncDisposable | |
| public partial class CopilotClient : IDisposable, IAsyncDisposable |
There was a problem hiding this comment.
This was intentional, as otherwise it should change its disposable implementation to accomodate derived types, and deriving from CopilotClient doesn't seem like an important scenario. @SteveSandersonMS, let me know if you disagree and I should revert this and instead change the IDisposable stuff to follow the disposable pattern.
There was a problem hiding this comment.
Sealing things that we don't explicitly have reasons to leave unsealed sounds good to me. Can always unseal later if we need.
- Add Central Package Management (Directory.Packages.props) with all package versions centralized - Add Directory.Build.props with shared properties (TargetFramework, ImplicitUsings, Nullable, TreatWarningsAsErrors) - Add nuget.config with single nuget.org source and package source mapping (required by CPM) - Add global.json specifying .NET 10 SDK (the library is still built with a net8.0 TFM) - Update all CI workflows from .NET 8.0.x to .NET 10.0.x - Enable XML documentation file generation (GenerateDocumentationFile) - Add XML doc comments to all non-generated public types and members - Add valid-value lists in XML docs for string properties with known values (e.g. PermissionRequest.Kind, ToolResultObject.ResultType) - Add #pragma warning disable CS1591 to generated files (SessionEvents.cs, Rpc.cs) and codegen scripts - Enable EmbedUntrackedSources, IncludeSymbols, SymbolPackageFormat - Enable ContinuousIntegrationBuild conditional on CI/TF_BUILD environment variables - Add PackageProjectUrl to package metadata - Add [EditorBrowsable(Never)] to obsolete GithubToken property - Upgrade analysis level and fix some diagnostics
dc1049d to
f4b9908
Compare