-
Notifications
You must be signed in to change notification settings - Fork 314
Merge | SqlCommand Execute Xml #3637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… link) # Conflicts: # src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs # src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs
There was a problem hiding this 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 continues the SqlCommand code organization effort by consolidating ExecuteXmlReader-related methods from platform-specific files into a shared partial class. The refactoring extracts XML reader functionality from both .NET Framework and .NET Core implementations, moving them to a common location for better maintainability and reduced code duplication.
- Consolidates ExecuteXmlReader methods into a shared partial class
- Removes duplicated XML reader implementation from platform-specific files
- Updates project files to include the new shared Xml partial class
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
SqlCommand.Xml.cs | New shared partial class containing all ExecuteXmlReader methods and supporting structures |
SqlCommand.NonQuery.cs | Minor formatting improvements to callback parameter handling |
SqlCommand.netfx.cs | Removes ExecuteXmlReader methods and ExecuteXmlReaderAsyncCallContext class |
SqlCommand.netcore.cs | Removes ExecuteXmlReader methods and ExecuteXmlReaderAsyncCallContext class |
Microsoft.Data.SqlClient.csproj (both) | Adds reference to the new shared Xml partial class |
CancellationTokenRegistration disposable, | ||
Guid operationId) | ||
{ | ||
base.Set(command,source, disposable); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after comma. Should be 'base.Set(command, source, disposable);'
Copilot uses AI. Check for mistakes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3637 +/- ##
==========================================
+ Coverage 65.17% 70.28% +5.10%
==========================================
Files 277 272 -5
Lines 60592 60124 -468
==========================================
+ Hits 39493 42256 +2763
+ Misses 21099 17868 -3231
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:
|
Description
In this installment of merging SqlCommand, the focus is on the ExecuteXmlReader methods and supporting structures. As with previous PRs in this series, a new partial has been introduced for the Xml methods. Every commit is a bite-sized merge of a single method or so.
The following methods were merged:
And also the ExecuteXmlReaderAsyncCallContext internal class
Issues
Continuation of work for #1261
Testing
Project still builds, CI should validate