Skip to content

Conversation

edwardneal
Copy link
Contributor

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, since spns 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:

  • CreatePhysicalSNIHandle
  • AssignPendingDNSInfo
  • CreateSessionHandle
  • FreeGcHandle
  • Dispose

Relates to #1261.

edwardneal added 29 commits May 16, 2025 19:36
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.
@edwardneal edwardneal requested a review from a team as a code owner June 5, 2025 06:51
@paulmedynski
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@paulmedynski paulmedynski requested review from mdaigle and benrr101 July 22, 2025 12:26
mdaigle
mdaigle previously approved these changes Jul 23, 2025
@benrr101
Copy link
Contributor

@edwardneal ahh I'm sorry I didn't look at this soon enough and now it needs a conflct resolved. I'd do it myself but then I can't approve it.

@edwardneal
Copy link
Contributor Author

Thanks @benrr101, I've just re-merged.

@benrr101
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

samsharma2700
samsharma2700 previously approved these changes Jul 28, 2025
Copy link
Contributor

@samsharma2700 samsharma2700 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for moving methods per commit!

mdaigle
mdaigle previously approved these changes Jul 29, 2025
@edwardneal edwardneal dismissed stale reviews from mdaigle and samsharma2700 via 2ed58de August 7, 2025 19:59
@edwardneal
Copy link
Contributor Author

This is now merged to address the conflicts from #3469. Could I have a CI run please?

@samsharma2700
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mdaigle mdaigle self-assigned this Aug 18, 2025
mdaigle
mdaigle previously approved these changes Aug 26, 2025
This allows TdsParser.netfx.cs to be deleted
@edwardneal
Copy link
Contributor Author

Merged to address the conflicts from #3535. This also allowed the removal of TdsParser.netfx.cs.

@benrr101
Copy link
Contributor

benrr101 commented Sep 2, 2025

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@benrr101
Copy link
Contributor

benrr101 commented Sep 3, 2025

@edwardneal I'm going to kick the failing tests again, but if the macos tests keeps failing, you might need to merge from main now that #3591 is merged

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.

6 participants