Skip to content

Fix API docs#4065

Draft
mdaigle wants to merge 3 commits intomainfrom
dev/mdaigle/validate-api-docs
Draft

Fix API docs#4065
mdaigle wants to merge 3 commits intomainfrom
dev/mdaigle/validate-api-docs

Conversation

@mdaigle
Copy link
Contributor

@mdaigle mdaigle commented Mar 18, 2026

Updates XML documentation comments and references throughout the codebase and documentation to improve accuracy and maintainability. It also makes some minor behavioral clarifications.

@mdaigle mdaigle added this to the 7.1.0-preview1 milestone Mar 18, 2026
Copilot AI review requested due to automatic review settings March 18, 2026 18:10
@github-project-automation github-project-automation bot moved this to To triage in SqlClient Board Mar 18, 2026
@mdaigle mdaigle changed the title Dev/mdaigle/validate api docs Validate api docs at build time Mar 18, 2026
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 improves the correctness of generated XML API documentation by fixing <include> snippet paths across several source files, correcting generic type/member references in snippet XML, and enabling XML doc generation for the main Microsoft.Data.SqlClient project.

Changes:

  • Fix relative <include file='...'> paths in several src/ files so snippet XML is resolved correctly during doc generation.
  • Update snippet XML references (notably for generic SqlVector<T> and generic retry logic method xrefs) and clarify SqlDataAdapter event documentation.
  • Enable XML documentation file generation in Microsoft.Data.SqlClient.csproj.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlVector.cs Fix <include> paths (and remove BOM) so SqlVector<T> docs pull from the correct snippet file.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlJson.cs Fix <include> paths so SqlJson docs pull from the correct snippet file.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlDbTypeExtensions.cs Fix <include> paths to the correct Microsoft.Data snippet location.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs Fix <include> paths for GetSqlJson / GetSqlVector documentation.
src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj Enable GenerateDocumentationFile for the main project.
doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml Correct SqlVector<T> cref usage to generic-arity form.
doc/snippets/Microsoft.Data.SqlClient/SqlDataAdapter.xml Clarify RowUpdating/RowUpdated behavior and fix event references + sample query spacing.
doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml Fix generic-method xref syntax and correct vector-type xref arity.
.github/prompts/ado-work-item-clarification.prompt.md Update prompt guidance to always query the ADO.Net project for work items.

You can also share your feedback on Copilot code review. Take the survey.

@codecov
Copy link

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.16%. Comparing base (00b77cd) to head (7d83aff).
⚠️ Report is 5 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (00b77cd) and HEAD (7d83aff). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (00b77cd) HEAD (7d83aff)
CI-SqlClient 2 0
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4065      +/-   ##
==========================================
- Coverage   75.42%   68.16%   -7.26%     
==========================================
  Files         280      275       -5     
  Lines       43834    66924   +23090     
==========================================
+ Hits        33060    45621   +12561     
- Misses      10774    21303   +10529     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 68.16% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mdaigle mdaigle changed the title Validate api docs at build time Fix api docs Mar 18, 2026
@mdaigle mdaigle changed the title Fix api docs Fix API docs Mar 18, 2026
@paulmedynski paulmedynski self-assigned this Mar 19, 2026
Copy link
Contributor

@paulmedynski paulmedynski left a comment

Choose a reason for hiding this comment

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

One question.

Output parameters (whether prepared or not) must have a user-specified data type. If you specify a variable length data type except vector, you must also specify the maximum <xref:Microsoft.Data.SqlClient.SqlParameter.Size%2A>.

For vector data types, the <xref:Microsoft.Data.SqlClient.SqlParameter.Size%2A> property is ignored. The size of the vector is inferred from the <xref:Microsoft.Data.SqlClient.SqlParameter.Value%2A> of type <xref:Microsoft.Data.SqlTypes.SqlVector%2A>.
For vector data types, the <xref:Microsoft.Data.SqlClient.SqlParameter.Size%2A> property is ignored. The size of the vector is inferred from the <xref:Microsoft.Data.SqlClient.SqlParameter.Value%2A> of type <xref:Microsoft.Data.SqlTypes.SqlVector%601>.
Copy link
Contributor

@paulmedynski paulmedynski Mar 19, 2026

Choose a reason for hiding this comment

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

Why did this change from %2A to %601, but not the one on line 3607 above? I'm guessing it's actually %60 (backtick) followed by 1, but that's pretty nasty if so. Can we not use backtick characters in XML?

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

Labels

None yet

Projects

Status: To triage

Development

Successfully merging this pull request may close these issues.

3 participants