Skip to content

Improve .NET SDK build infrastructure and documentation#643

Open
stephentoub wants to merge 1 commit intogithub:mainfrom
stephentoub:stoub/updatedotnetbuild
Open

Improve .NET SDK build infrastructure and documentation#643
stephentoub wants to merge 1 commit intogithub:mainfrom
stephentoub:stoub/updatedotnetbuild

Conversation

@stephentoub
Copy link
Collaborator

  • 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

@stephentoub stephentoub requested a review from a team as a code owner March 3, 2026 17:41
Copilot AI review requested due to automatic review settings March 3, 2026 17:41
@stephentoub stephentoub enabled auto-merge March 3, 2026 17:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
public sealed partial class CopilotClient : IDisposable, IAsyncDisposable
public partial class CopilotClient : IDisposable, IAsyncDisposable

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants