-
Notifications
You must be signed in to change notification settings - Fork 292
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
Merge | LocalDBAPI #3047
base: main
Are you sure you want to change the base?
Merge | LocalDBAPI #3047
Conversation
…ly loaded on Windows
…ot* want to try to decompose stuff in TdsParser to remove LocalDBAPI references on non-windows systems.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalDBAPI.Windows.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalDBAPI.Windows.cs
Outdated
Show resolved
Hide resolved
private delegate int LocalDBFormatMessageDelegate(int hrLocalDB, uint dwFlags, uint dwLanguageId, StringBuilder buffer, ref uint buflen); | ||
|
||
#if NETFRAMEWORK | ||
private static LocalDBCreateInstanceDelegate LocalDBCreateInstance |
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.
Could we port this functionality to .NET Core as part of the merge please? It's a slightly odd behavioural difference which triggered #2888.
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.
Does that basically mean include in both netfx and netcore and wire it up the same way as in netfx? I'd be happy to, but it's also a functional change that I don't really want to take as part of the merge tasks.
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.
Yes, that's correct. It doesn't need to happen as part of this PR, but it'd be good to merge smaller behavioural quirks like this too.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3047 +/- ##
==========================================
+ Coverage 72.66% 72.76% +0.10%
==========================================
Files 283 280 -3
Lines 58975 58895 -80
==========================================
+ Hits 42853 42856 +3
+ Misses 16122 16039 -83
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description: This is one more PR that targets merging another file between the netfx project and the netcore project. This time, it's the LocalDBAPI. This one was a bit complicated because the files were split differently between netfx and netcore. The way I approached it was to merge the operative code into the LocalDBAPI.Windows file and merge the inoperative code to the LocalDBAPI.Unix file. The Windows file will be pulled in on both netfx and netcore projects, with it being conditionally included on netcore if the build is for Windows. The Unix file will be pulled in on netcore only, being conditionally included if the build is for Unix. Otherwise, this is a fairly standard merge with netfx having extra stuff that is
#if
'd in.please note: cleanup to remove monitor sections, etc, is coming in a later PR.
Testing: There miiiiight be some issues with unix build, I might not have fully vetted it on unix.