NotificationsYou must be signed in to change notification settings
Fork311
Star927

Merge | SessionHandle.*.cs#3247

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

benrr101 merged 1 commit intomainfromdev/russellben/merge/sessionhandle

Apr 2, 2025

Merged

Merge | SessionHandle.*.cs#3247

benrr101 merged 1 commit intomainfromdev/russellben/merge/sessionhandle

Apr 2, 2025

Conversation

Copy link

Contributor

benrr101 commentedMar 26, 2025

Description: Super duper easy, moving the SessionHandle class implementations from the netcore project to the common project. Classes are now wrapped with #if NET

There's a very small difference between the two files, but as it stands today we don't have the right solution for #if defining the OS. This will be addressed later, once all classes have been merged.

Testing: Literally no functional changes. Tests should breeze through unless I goofed on building for Unix.

Move sessionhandle files to common project

9588f39

benrr101 added the Common Project 🚮Things that relate to the common project project label

Mar 26, 2025

benrr101 requested review froma team andCopilot

March 26, 2025 23:18

CopilotAI reviewed

Mar 26, 2025

View reviewed changes

Copy link

Contributor

CopilotAI left a 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 moves the SessionHandle class implementations from the netcore project into the common project and wraps the classes in conditional compilation blocks (#if NET) to support portability between Windows and Unix.

Moves implementations of SessionHandle from netcore-specific files to a common project
Introduces #if NET directives in both Windows and Unix source files as a temporary solution

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/SessionHandle.netcore.Windows.cs	Added "#if NET" wrapping to support conditional compilation for Windows-only code
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SessionHandle.netcore.Unix.cs	Added "#if NET" wrapping to support conditional compilation for Unix-specific code

Files not reviewed (1)

src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj: Language not supported

Comments suppressed due to low confidence (2)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SessionHandle.netcore.Windows.cs:5

[nitpick] The conditional compilation symbol 'NET' is ambiguous given the multiple .NET versions available; consider using a more descriptive symbol (e.g., NETCORE or NET5_0_OR_GREATER) to clarify its intent.

#if NET

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SessionHandle.netcore.Unix.cs:5

[nitpick] The conditional compilation symbol 'NET' is ambiguous given the multiple .NET versions available; consider using a more descriptive symbol (e.g., NETCORE or NET5_0_OR_GREATER) to clarify its intent.

#if NET

Copy link

codecovbot commentedMar 27, 2025•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.82%. Comparing base(1247ca4) to head(9588f39).
Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@##             main    #3247      +/-   ##==========================================+ Coverage   72.69%   72.82%   +0.13%==========================================  Files         303      300       -3       Lines       59718    59611     -107     ==========================================  Hits        43414    43414+ Misses      16304    16197     -107