Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add async methods in System.Data.Common, implement IAsyncDisposable#37877

Merged
roji merged 1 commit into
dotnet:masterfrom
roji:AsynchronizeSystemData
May 30, 2019
Merged

Add async methods in System.Data.Common, implement IAsyncDisposable#37877
roji merged 1 commit into
dotnet:masterfrom
roji:AsynchronizeSystemData

Conversation

@roji

@roji roji commented May 22, 2019

Copy link
Copy Markdown
Member

Closes #35012

@roji roji requested review from ajcvickers and divega May 22, 2019 20:33
Comment thread src/System.Data.Common/src/System/Data/Common/DbCommand.netcoreapp.cs Outdated

@ajcvickers ajcvickers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @divega to double check the API surface.

Comment thread src/System.Data.Common/src/System/Data/Common/DbCommand.netcoreapp.cs Outdated
Comment thread src/System.Data.Common/src/System/Data/Common/DbDataReader.netcoreapp.cs Outdated
Comment thread src/System.Data.Common/src/System/Data/Common/DbTransaction.netcoreapp.cs Outdated
Comment thread src/System.Data.Common/src/System/Data/Common/DbConnection.netcoreapp.cs Outdated
Comment thread src/System.Data.Common/ref/System.Data.Common.netcoreapp.cs Outdated
@roji

roji commented May 23, 2019

Copy link
Copy Markdown
Member Author

ping @divega for an extra set of eyes

@divega divega left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can recall, when we did #36123 we learned that we don’t need to do the dance with partials and *.netcoreapp.cs files in product code, I think because we are no longer building the package that targets other TFMs from the source in master.

If this is true, my preference would be to consolidate the code files.

Other than that, everything looks good to me.

@roji roji force-pushed the AsynchronizeSystemData branch from c481050 to 6259b22 Compare May 30, 2019 10:42
@roji

roji commented May 30, 2019

Copy link
Copy Markdown
Member Author

@divega you're right, I did a test build in #38051 and it works - seems like the issue was with the tests in #36123, and we currently don't have any in this PR. Rebased, squashed and removed the netcoreapp special-casing.

@roji roji merged commit 22dd87e into dotnet:master May 30, 2019
@roji roji deleted the AsynchronizeSystemData branch May 30, 2019 13:23
@karelz karelz added this to the 3.0 milestone Jul 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add missing async methods in System.Data.Common and implement IAsyncDisposable

6 participants