Skip to content

Cleanup | SNI Native Wrapper - #3156

Merged
benrr101 merged 12 commits into
mainfrom
dev/russellben/cleanup/sni
Mar 10, 2025
Merged

Cleanup | SNI Native Wrapper#3156
benrr101 merged 12 commits into
mainfrom
dev/russellben/cleanup/sni

Conversation

@benrr101

Copy link
Copy Markdown
Contributor

This is a replacement of #3056 that will give us the complete CI

Description: This is a followup to the SNI Native Wrapper code merge. Each commit in this PR is an atomic change, so if the PR gets too cluttered to review all up, it can be stepped through one commit at a time. Most of these changes are just renaming and moving code around. One larger change is removing a bunch of private methods that were no longer necessary after extracting the DLL imports (as mentioned by @edwardneal).

Testing: There aren't really any functional changes here, so CI pass should be sufficient validation.

@benrr101benrr101 added Code Health 💊 Issues/PRs that are targeted to source code quality improvements. Common Project 🚮 Things that relate to the common project project labels Feb 13, 2025
@benrr101benrr101 added this to the 7.0-preview1 milestone Feb 13, 2025
@benrr101
benrr101 requested a review from a teamFebruary 13, 2025 21:55
@benrr101benrr101 mentioned this pull request Feb 13, 2025
@benrr101

Copy link
Copy Markdown
ContributorAuthor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@paulmedynski
paulmedynski self-requested a review February 20, 2025 19:55
@codecov

codecovBot commented Feb 20, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 92.06349% with 15 lines in your changes missing coverage. Please review.

Project coverage is 72.52%. Comparing base (87b5435) to head (ac9be84).
Report is 4 commits behind head on main.

Files with missing linesPatch %Lines
...Client/src/Interop/Windows/Sni/SniNativeWrapper.cs95.41%5 Missing ⚠️
...ient/src/Microsoft/Data/SqlClient/SqlDependency.cs75.00%4 Missing ⚠️
...osoft/Data/SqlClient/LocalDb/LocalDbApi.Windows.cs0.00%2 Missing ⚠️
...Data/SqlClient/TdsParserStateObject.Multiplexer.cs0.00%2 Missing ⚠️
...nt/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs87.50%1 Missing ⚠️
...ft/Data/Sql/SqlDataSourceEnumeratorNativeHelper.cs85.71%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3156 +/- ##
==========================================
- Coverage 72.57% 72.52% -0.05% 
==========================================
Files 288 289 +1 Lines 59526 59503 -23 ==========================================
- Hits 43201 43156 -45 - Misses 16325 16347 +22 
FlagCoverage Δ
addons92.58% <ø> (ø)
netcore75.08% <90.29%> (-0.01%)⬇️
netfx71.16% <91.71%> (-0.06%)⬇️

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.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@benrr101
benrr101force-pushed the dev/russellben/cleanup/sni branch 2 times, most recently from 5aa8c2c to bc5bc5bCompareMarch 4, 2025 01:35
Comment threadsrc/Microsoft.Data.SqlClient/src/Interop/Windows/Sni/SniNativeWrapper.cs Outdated
@benrr101
benrr101force-pushed the dev/russellben/cleanup/sni branch from 12614c8 to ac9be84CompareMarch 10, 2025 18:04
@benrr101
benrr101 merged commit 1e59b88 into mainMar 10, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Code Health 💊Issues/PRs that are targeted to source code quality improvements.Common Project 🚮Things that relate to the common project project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@benrr101@mdaigle@paulmedynski