- Notifications
You must be signed in to change notification settings - Fork311
Merge | SqlFileStream (Opt 1)#2898
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
Merge | SqlFileStream (Opt 1)#2898
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedOct 7, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #2898 +/- ##==========================================+ Coverage 71.92% 72.24% +0.32%========================================== Files 294 291 -3 Lines 60342 59769 -573 ==========================================- Hits 43398 43180 -218+ Misses 16944 16589 -355
Flags with carried forward coverage won't be shown.Click here to find out more. ☔ View full report in Codecov by Sentry. |
src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Seems like a lot of hassle for little benefit :/
* Fix annoyance with failing unit test setup
* Use variable versions* Remove output type from common project
* Normalize the path *once*
* Fix method signatures (modifier order, argument formatting)* Touching up comments
9e157a5
toefad644
Comparesrc/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlTypes/SqlFileStream.Windows.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlFileStream.Windows.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Can you please take a look at conflicts, thanks! |
2e163b2
intodotnet:mainUh oh!
There was an error while loading.Please reload this page.
Description: This PR aims to merge
SqlFileStream
from the netfx and netcore projects. The strategy for this merge is to use the Interop.* libraries from the netcore project to enable merging the classes. Therefore, the code for this class is mostly based on the netcore implementation with a handful of#if NETFRAMEWORK
blocks to add overrides for netfx behavior.The commits are broken up into fairly digestible chunks that explain each step along the process. I'd recommend reviewing the PR by stepping through the commits and seeing if you agree with the changes. Nevertheless, this PR has a lot of code cleanup changes at the end that make the overall PR fairly large. In anticipation of pushback on this, I will offer a separate PR that only contains the merge changes. If that one is accepted, I'll still send out the cleanup change on a separate PR. But, I think this option would be faster overall.
Testing: The existing manual tests for SqlFileStream were used to validate behavior.