-
Notifications
You must be signed in to change notification settings - Fork 317
Merge | SqlCommand NonQuery #3630
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
Merge | SqlCommand NonQuery #3630
Conversation
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 merges ExecuteNonQuery-related APIs from platform-specific files into a common shared file as part of the ongoing SqlCommand consolidation effort. It creates a new SqlCommand.NonQuery.cs partial class containing all ExecuteNonQuery methods and removes duplicate code from both .NET Framework and .NET Core implementations.
- Consolidates ExecuteNonQuery APIs into a single shared implementation
- Removes platform-specific duplicates from both netfx and netcore files
- Updates class declaration to be sealed consistently across files
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SqlCommand.NonQuery.cs | New shared partial class containing merged ExecuteNonQuery APIs |
| SqlCommand.Scalar.cs | Updates class to be sealed partial and improves variable naming |
| SqlCommand.netfx.cs | Removes duplicate ExecuteNonQuery implementations moved to shared file |
| SqlCommand.netcore.cs | Removes duplicate ExecuteNonQuery implementations moved to shared file |
| Microsoft.Data.SqlClient.csproj (netfx) | Adds reference to new shared NonQuery partial file |
| Microsoft.Data.SqlClient.csproj (netcore) | Adds reference to new shared NonQuery partial file |
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3630 +/- ##
==========================================
- Coverage 66.13% 66.04% -0.09%
==========================================
Files 276 277 +1
Lines 60765 60554 -211
==========================================
- Hits 40184 39995 -189
+ Misses 20581 20559 -22
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 the SqlCommand merge PRs, we merge the ExecuteNonQuery APIs. This includes adding a partial SqlCommand.NonQuery, merging the ExecuteNonQueryAsyncCallContext subclass and merging the following methods:
I tried my best to avoid functional changes when making stylistic changes. But I can be a bit heavy handed with the stylistic changes from time to time.
Issues
Continuation of work on #1261
Testing
Code still compiles, CI will validate