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

Remove SqlCommand code paths for context connections #2996

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

edwardneal
Copy link
Contributor

Contributes to #2838. PR 2/7.

This doesn't need to be merged before 6.0. It removes the core SqlDataReaderSmi type, and the code paths in SqlCommand.ExecuteNonQuery and SqlCommand.ExecuteReader which reference it. There are also a handful of Stream and TextReader derivatives which are only used in those contexts.

One point of note is in SqlDataRecord. InOutOfProcHelper.InProc will always be false, so I've trimmed the dead code in the if condition. As a result, _recordContext will always be null, and null will always propagate to the context parameter in ValueUtilsSmi. I plan to clean this up in the next PR.

If this is too large to review, I can partially revert some of these removals.

Copy link

codecov bot commented Nov 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.52%. Comparing base (4a9b070) to head (fa01093).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2996      +/-   ##
==========================================
+ Coverage   72.45%   73.52%   +1.06%     
==========================================
  Files         288      283       -5     
  Lines       59498    58652     -846     
==========================================
+ Hits        43110    43122      +12     
+ Misses      16388    15530     -858     
Flag Coverage Δ
addons 92.58% <ø> (ø)
netcore 75.42% <ø> (+0.05%) ⬆️
netfx 72.43% <100.00%> (+1.49%) ⬆️

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant