Skip to content

Merge | SessionHandle, TdsParserStateObject #3341

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

Merged
merged 4 commits into from
May 16, 2025

Conversation

edwardneal
Copy link
Contributor

This starts to merge TdsParserStateObject. I've broken this down commit-by-commit for simplicity, but there are three changes here:

  1. Where there are methods in TdsParserStateObject's netcore and netfx-specific files which are identical, move these to the shared TdsParserStateObject.cs file.
  2. Removing a SessionHandle ref struct definition from the netfx-specific file. This is replaced by the existing SessionHandle.Windows.cs file's definition.
  3. Convert the Status, SessionId and SessionHandle properties to abstract implementations in TdsParserStateObjectNative in netfx, to match netcore.

Contributes to #1261.

SessionHandle has a parallel in PacketHandle. This is treated as an alias of IntPtr in netfx, and a type in its own right in netcore. Dealing with this will probably need a PR in its own right, and it'll unblock a number of methods which are identical but which deal with PacketHandles.

@benrr101
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@benrr101 benrr101 added the Common Project 🚮 Things that relate to the common project project label May 12, 2025
@benrr101 benrr101 requested a review from a team May 12, 2025 16:51
@apoorvdeshmukh apoorvdeshmukh requested a review from a team May 13, 2025 17:12
Copy link

codecov bot commented May 15, 2025

Codecov Report

Attention: Patch coverage is 80.84112% with 41 lines in your changes missing coverage. Please review.

Project coverage is 58.67%. Comparing base (14bee54) to head (715e71c).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs 80.40% 39 Missing ⚠️
...osoft/Data/SqlClient/TdsParserStateObject.netfx.cs 85.71% 1 Missing ⚠️
...osoft/Data/SqlClient/TdsParserStateObjectNative.cs 66.66% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (14bee54) and HEAD (715e71c). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (14bee54) HEAD (715e71c)
addons 1 0
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3341      +/-   ##
==========================================
- Coverage   65.07%   58.67%   -6.40%     
==========================================
  Files         298      293       -5     
  Lines       65515    65059     -456     
==========================================
- Hits        42634    38176    -4458     
- Misses      22881    26883    +4002     
Flag Coverage Δ
addons ?
netcore 62.79% <80.50%> (-5.55%) ⬇️
netfx 59.61% <80.28%> (-6.56%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@benrr101 benrr101 merged commit 75d5277 into dotnet:main May 16, 2025
237 checks passed
@edwardneal edwardneal deleted the merge/tdsparserstateobject-pt1 branch May 16, 2025 16:56
@paulmedynski paulmedynski added this to the 6.1-preview2 milestone Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Common Project 🚮 Things that relate to the common project project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants