-
Notifications
You must be signed in to change notification settings - Fork 308
Merge | Port #528 and #2091 to netfx #3376
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
Conversation
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
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.
A couple of spurious ? checks.
I didn't dig into the full logic that was added/modified - I don't have enough experience with SqlCommand for that yet.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs
Outdated
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Since this is porting changes into netfx, I'm onboard with these changes
Description
This ports an earlier set of memory usage reductions from netcore to netfx.
Issues
Relates to #1261.
Ports #528 and #2091.
Testing
Unit tests pass locally. I don't see anything specifically targeting these changes, so I assume that they're covered by the overall test suite - they're fairly frequently exercised.