Merge | PacketHandle.*.cs #3241
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR moves the PacketHandle.netcore.Windows.cs and PacketHandle.netcore.Unix.cs files from the netcore project to the common project while updating file names to clarify that they only apply to netcore.
- Added conditional compilation directives (#if NET / #endif) to both files
- Reformatted comments and updated multi-line lambda formatting for readability
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.netcore.Unix.cs | Added #if NET wrapper and updated comment formatting and lambda method style |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.netcore.Windows.cs | Added #if NET wrapper, updated comment formatting, lambda method style, and reordered field declarations |
Files not reviewed (1)
- src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj: Language not supported
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.netcore.Windows.cs:28
- The reordered field declarations (NativePacket before NativePointer) may affect memory layout if the struct relies on a specific field order. Please confirm that this change is intentional and does not impact any layout-dependent logic.
public readonly SNIPacket NativePacket;
src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3241 +/- ##
==========================================
+ Coverage 72.69% 72.80% +0.10%
==========================================
Files 303 300 -3
Lines 59718 59611 -107
==========================================
- Hits 43414 43398 -16
+ Misses 16304 16213 -91
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:
|
|
The reason there are two files is that at the time the common practice in this library was to avoid using compiler directives and use msbuild conditions. Since then I think everyone involved has got tired of the complexity involved and the amount of difficulty debugging msbuild so it might be worth considering just combining the two files and using compiler directives. /cc @David-Engel |
|
@Wraith2 yes, the eventual plan is to use compiler directives for the different OS builds. Not there yet, but it's in the plan. |
Description: Very simple move of PacketHandle.Windows.cs and PacketHandle.Unix from the netcore project to the common project. File names updated to reflect that they only apply to netcore.
Testing: Literally no functional changes, just moving files.