Merge | DbConnectionOptions (The Rest Of It) #3388
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates the platform-specific partial classes for DbConnectionOptions into a single common file using preprocessor directives, removing the need for separate .netfx and .netcore files. Key changes include merging platform-specific code into DbConnectionOptions.cs, removing the obsolete partial files, and updating the project files to reflect these deletions.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.cs | Merged platform-specific code with preprocessor directives; added a TODO comment regarding parser extraction. |
| src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.netfx.cs | Removed as its contents are now merged into the common DbConnectionOptions.cs file. |
| src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.netcore.cs | Removed as its contents are now merged into the common DbConnectionOptions.cs file. |
| src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient.csproj | Updated to remove reference to the .netfx partial class file. |
| src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient.csproj | Updated to remove reference to the .netcore partial class file. |
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
From a pure merge perspective, it looks good to me. Interested in some of the additional improvements that @edwardneal has suggested.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3388 +/- ##
==========================================
- Coverage 68.33% 68.20% -0.14%
==========================================
Files 301 299 -2
Lines 65631 65406 -225
==========================================
- Hits 44849 44608 -241
- Misses 20782 20798 +16
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 the previous PR, we moved the debug-only code for DbConnectionOptions into its own partial class and renamed the remaining partials to follow the merge pattern (ie, *.netcore.cs and *.netfx.cs). This PR completes the process by moving the platform-specific partials into the common DbConnectionOptions.cs file. The code is wrapped in pre-processor directives to ensure they are only compiled on the appropriate platforms.
Issues
Continues work towards #1261
Testing
Code has moved but the functionality has not changed. Project still builds, and CI should be enough for validation.