-
Notifications
You must be signed in to change notification settings - Fork 311
Merge | TdsParserStateObject lifetime-related methods #3394
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
base: main
Are you sure you want to change the base?
Merge | TdsParserStateObject lifetime-related methods #3394
Conversation
Also move ReadSyncOverAsync down in netcore's TdsParserStateObjectNative to aid later merge
Also reorder member in TdsParserStateObjectNative to simplify later merge
This also allows _writePacketCache to be migrated to TdsParserStateObjectNative in netfx.
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3394 +/- ##
==========================================
- Coverage 63.51% 60.03% -3.48%
==========================================
Files 293 285 -8
Lines 63810 63405 -405
==========================================
- Hits 40526 38067 -2459
- Misses 23284 25338 +2054
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:
|
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
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.
Overall, love it - sorry it took so long to get it looked at. Couple questions, but looking forward to merging.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs
Outdated
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
@@ -408,28 +414,17 @@ internal void Connect(ServerInfo serverInfo, | |||
} | |||
} | |||
|
|||
_serverSpn = null; |
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.
duplicate of line 400
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.
Thanks - removed from netfx and netcore.
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Description
We're now merging the lifetime-related methods on TdsParserStateObject in netfx, making them abstract and pushing them down to the derived TdsParserStateObjectNative class. This also handles some slightly unusual logic in
CreatePhysicalSNIHandle
, sincespns
is a string in netfx and a string array in netcore. I've needed to merge this type handling too.Commit 469d840 onwards moves method-by-method. I've moved:
Relates to #1261.