Skip to content
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 PoolWaitHandles, DbConnectionPoolIdentity, removed DbBuffer #2390

Conversation

edwardneal
Copy link
Contributor

Contributes to #1261.

This is a little more involved than the previous merges. PoolWaitHandles and DbConnectionPoolIdentity in .NET Framework currently use direct PInvoke to wait on named objects and get the Windows identity details. I've merged these with their .NET Core counterparts, which behave the same way.

Doing so also removes the last reference to DbBuffer, which was only present in the .NET Framework project. With those references gone, I've removed this too. This contributes to three quarters of the deletions - the PR's a fair bit smaller than it looks!

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

Attention: Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.

Project coverage is 72.04%. Comparing base (619fa74) to head (b645fea).
Report is 5 commits behind head on main.

Files Patch % Lines
...rc/Microsoft/Data/ProviderBase/DbConnectionPool.cs 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2390      +/-   ##
==========================================
+ Coverage   71.66%   72.04%   +0.38%     
==========================================
  Files         308      305       -3     
  Lines       62297    61804     -493     
==========================================
- Hits        44646    44529     -117     
+ Misses      17651    17275     -376     
Flag Coverage Δ
addons 92.90% <ø> (ø)
netcore 75.91% <86.66%> (-0.01%) ⬇️
netfx 70.22% <76.00%> (+0.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.

@David-Engel David-Engel added this to the 6.0-preview1 milestone Mar 30, 2024
@DavoudEshtehari
Copy link
Member

@edwardneal Can you address the conflicts, please?

@edwardneal
Copy link
Contributor Author

Thanks @DavoudEshtehari. I've handled the merge conflicts, and the test failure seems to be unrelated to me.

@DavoudEshtehari
Copy link
Member

Given these changes, I believe the DbConnectionPool.NetCoreApp.cs file can be merged with the other files to make it simpler to follow. It's not a critical change request, but it would be nice to have.

@edwardneal
Copy link
Contributor Author

I agree. I've moved the method into DbConnectionPool.cs and guarded it behind #if NET6_0_OR_GREATER to make the later merge simpler.

@DavoudEshtehari DavoudEshtehari added the ➕ Code Health Changes related to source code improvements label Aug 21, 2024
@DavoudEshtehari DavoudEshtehari merged commit 6d33e9d into dotnet:main Aug 21, 2024
131 checks passed
@edwardneal edwardneal deleted the issue-1261-dbconnectionpoolidentity-dbbuffer branch August 22, 2024 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
➕ Code Health Changes related to source code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants