Documentation | Correct SqlDataRecord, SqlMetaData documentation - #4440
Documentation | Correct SqlDataRecord, SqlMetaData documentation#4440edwardneal wants to merge 3 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
This pull request has been marked as stale due to inactivity for more than 30 days. If you would like to keep this pull request open, please provide an update or respond to any comments. Otherwise, it will be closed automatically in 7 days. |
github-actions
Bot
added
the
Stale
|
This PR is not stale. |
github-actions
Bot
removed
the
Stale
priyankatiwari08
left a comment
priyankatiwari08
left a comment
There was a problem hiding this comment.
The direction of this change is right: the old samples showed SqlDataRecord being produced inside a [SqlProcedure] and pushed through SqlContext.Pipe.Send, which is a SQL CLR / System.Data.SqlClient-era pattern that Microsoft.Data.SqlClient.Server does not actually support. Rewriting them as a client-side TVP streaming sample over IEnumerable<SqlDataRecord> reflects the real supported usage, and the added <remarks> on both types are a genuine improvement.
One blocking issue and two smaller ones are inline. The cref form is the important one — as written those links will not resolve in the generated docs.
Additional notes without a diff anchor:
- Both XML files lost their UTF-8 BOM in this change (
\ufeff<docs>-><docs>). Worth confirming the docs ingestion tooling is BOM-agnostic before merging, since every other file underdoc/snippets/still carries one. - In
SqlMetaData.xmlthe new<remarks>block is placed after<example>. Elsewhere indoc/snippets/the ordering is<summary>,<remarks>,<example>. Not functionally significant, but inconsistent with the surrounding files.
| <remarks> | ||
| <para> | ||
| This class is used together with <see cref="T:Microsoft.SqlServer.Server.SqlPipe" /> to send result sets to the client from managed code stored-procedures. | ||
| This class describes a single row of a table-valued parameter. Construct one from an array of <see cref="T:Microsoft.Data.SqlClient.Server.SqlMetaData" /> objects which describe the column metadata of the record, populate it with the <b>Set<Type></b> methods and assign it (or an <see cref="T:System.Collections.Generic.IEnumerable{Microsoft.Data.SqlClient.Server.SqlDataRecord}" /> for multiple rows) to <see cref="P:Microsoft.Data.SqlClient.SqlParameter.Value" />. |
There was a problem hiding this comment.
These generic cref values will not resolve.
Files under doc/snippets/**/*.xml are raw XML consumed directly by the docs pipeline — they are not compiler-generated doc comments, so the C#-style {...} generic argument syntax is never translated into DocID form. The convention already established in this folder is backtick arity, e.g. cref="T:System.Collections.Generic.ICollection1"inSqlBatchCommandCollection.xmlandSqlBatch.xml`.
Applies to both occurrences in this hunk (this line and the IEnumerable{SqlDataRecord} reference a few lines below). Please switch to the IEnumerable`1 form, or drop the cref and use plain <c>IEnumerable<SqlDataRecord></c> if the link isn't needed.
There was a problem hiding this comment.
I've used the latter, but I'd like to be able to reference the generic type directly. Happy to leave as is, but do you know if <xref:System.Collections.Generic.IEnumerable``1[Microsoft.Data.SqlClient.Server.SqlDataRecord]> will work? It certainly doesn't in Visual Studio, but I don't know whether another layer of translation between this and the released package patches these up.
github-project-automation
Bot
moved this from Backlog
to Waiting for customer
in SqlClient Board
* Correct comments in SqlDataRecord.cs code (and identical snippet in docs.) * Grammar fix in SqlMetaData.xml. * Place summary/remarks/example tags in the correct order. * Use correct syntax for a reference to IEnumerable<SqlDataRecord>.
cheenamalhotra
left a comment
cheenamalhotra
left a comment
There was a problem hiding this comment.
Thanks for taking care of this!
4 participants
Description
The existing documentation for both types is located here:
SqlDataRecordSqlMetaDataThese pages are incorrect - they refer to the
SqlContextand theSqlPipetypes, which have never existed. There's also an open issue which asks for more information on whether it's valid for users to reuseSqlDataRecordinstances.This PR corrects the documentation for these two types, addressing #1805 in the process.
It might also be worthwhile backporting this to the other release branches, this is just documenting their existing behaviour.
Issues
Fixes #1805.
Testing
This isn't a functional change - just a change to documentation.