Add API documentation generation for the Logging package#4021
Add API documentation generation for the Logging package#4021apoorvdeshmukh wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds XML documentation support to the Microsoft.Data.SqlClient.Extensions.Logging package by enabling compiler-generated XML docs, preserving a full “untrimmed” copy for doc workflows, and trimming the packaged IntelliSense XML to remove <remarks>/<example> content.
Changes:
- Added XML doc comments to public APIs in
SqlClientEventSource. - Updated
Logging.csprojto generate XML documentation and added build targets to copy untrimmed docs toartifacts/doc/<tfm>/and then trim the in-place XML file. - Minor project metadata formatting tweaks (e.g., package description wrapping).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient.Extensions/Logging/src/SqlClientEventSource.cs | Adds/updates XML documentation comments across public APIs for the Logging EventSource. |
| src/Microsoft.Data.SqlClient.Extensions/Logging/src/Logging.csproj | Enables XML doc generation and introduces post-build targets to preserve and trim XML docs for packaging/IntelliSense. |
src/Microsoft.Data.SqlClient.Extensions/Logging/src/SqlClientEventSource.cs
Show resolved
Hide resolved
| // Do not change the first 4 arguments in this Event writer as OpenTelemetry and ApplicationInsight are relating to the same format, | ||
| // unless you have checked with them and they are able to change their design. Additional items could be added at the end. |
There was a problem hiding this comment.
Typo in the comment: the product name is "Application Insights" (plural). Consider also rephrasing "are relating" to something grammatically correct (e.g., "are related") while you're touching this comment.
paulmedynski
left a comment
There was a problem hiding this comment.
Lookg good overall! Just a few comments/questions.
src/Microsoft.Data.SqlClient.Extensions/Logging/src/Logging.csproj
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient.Extensions/Logging/src/SqlClientEventSource.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient.Extensions/Logging/src/SqlClientEventSource.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient.Extensions/Logging/src/SqlClientEventSource.cs
Show resolved
Hide resolved
| <Exec | ||
| Command="$(PowerShellCmd) -NonInteractive -ExecutionPolicy Unrestricted -Command "$(ToolsDir)intellisense\TrimDocs.ps1 -inputFile '$(DocumentationFile)' -outputFile '$(DocumentationFile)'"" /> |
There was a problem hiding this comment.
The Exec command invokes TrimDocs.ps1 using an unquoted script path ($(ToolsDir)intellisense\TrimDocs.ps1) inside -Command. If the repo path (and therefore $(ToolsDir)) contains spaces, PowerShell will split the path and fail to locate/execute the script. Quote the script path (and ideally use the call operator & or -File) so the target is robust for local builds in paths with spaces.
|
|
||
| #region Write Events | ||
| // Do not change the first 4 arguments in this Event writer as OpenTelemetry and ApplicationInsight are relating to the same format, | ||
| // Do not change the first 4 arguments in this Event writer as OpenTelemetry and ApplicationInsight are relating to the same format, |
There was a problem hiding this comment.
Minor grammar issue in the comment: "are relating" is ungrammatical here (should be e.g. "relate" / "are related"). Since this line was touched, consider fixing the wording to avoid carrying the typo forward.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #4021 +/- ##
==========================================
- Coverage 72.36% 65.06% -7.30%
==========================================
Files 287 282 -5
Lines 43149 66073 +22924
==========================================
+ Hits 31223 42988 +11765
- Misses 11926 23085 +11159
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mdaigle
left a comment
There was a problem hiding this comment.
I may be missing something, but I grabbed the npkg file from the PR-package pipeline and it still includes remarks comments. I also do not see an untrimmed artifact uploaded to the pipeline artifacts or included in the package.
MDS package has untrimmed documentation xml files under |
| Select the correct PowerShell executable for the current OS so that the | ||
| TrimDocsForIntelliSense target below does not need to duplicate the command. | ||
| --> | ||
| <PropertyGroup> |
There was a problem hiding this comment.
We will want somewhere that all projects can share it, like src/Directory.Build.props.
| <Exec | ||
| Command="$(PowerShellCmd) -NonInteractive -ExecutionPolicy Unrestricted -Command "$(ToolsDir)intellisense\TrimDocs.ps1 -inputFile '$(DocumentationFile)' -outputFile '$(DocumentationFile)'"" /> |
Description
Adds XML documentation comments to all public APIs in Microsoft.Data.SqlClient.Extensions.Logging and configures the project to generate, preserve, and trim XML documentation — following the same patterns used by the main Microsoft.Data.SqlClient package.
Issues
AB#42969
Changes
Added
<GenerateDocumentationFile>true</GenerateDocumentationFile>so the compiler generates an XML doc file alongside the assembly. SDK Pack automatically includes it in the NuGet package underlib/<tfm>/.Added
SaveUntrimmedDocstarget — copies the full (untrimmed) XML documentation toartifacts/doc/<tfm>/after build, preserving<remarks>and<example>content for public documentation workflows (e.g., API Drop → Content CI → dotnet/sqlclient-api-docs).Added TrimDocsForIntelliSense target — runs
TrimDocs.ps1to strip and nodes from the in-place XML file so the NuGet package ships a cleaner IntelliSense experience, matching how the main MDS package handles this.Added
<summary>, <param>, <typeparam>, <returns>,and<remarks>tags to all ~80+ public members across:SqlClientEventSource.cs — class-level docs with and , all Enable/Disable methods, Trace overloads (with and without if guards), Scope enter/leave methods, Execution Trace, Notification Trace/Scope, Pooler Trace/Scope, Advanced Trace/Scope/Bin/Error, Correlation Trace, State Dump, SNI Trace/Scope, and all WriteEvent methods
Notes
No pipeline changes needed: SDK Pack automatically includes the XML doc file in the NuGet package when
GenerateDocumentationFileis set. Both the CI pipeline (pack-logging-package-ci-job.yml) and the OneBranch official pipeline use dotnet pack, which handles this natively.