Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Add IAsyncDisposable implementations to refs #34898

Merged
merged 1 commit into from
Jan 28, 2019

Conversation

stephentoub
Copy link
Member

Apparently when I added the DisposeAsync methods to the refs, I didn't actually mark the corresponding types as implementing the interface. This fixes that.

cc: @ahsonkhan

Apparently when I added the DisposeAsync methods to the refs, I didn't actually mark the corresponding types as implementing the interface.  This fixes that.
@davidfowl
Copy link
Member

Don't we have tools that find ref/def mismatches?

@stephentoub
Copy link
Member Author

Don't we have tools that find ref/def mismatches?

We do, but the methods are there in both. I don't believe it looks at interfaces themselves. @ericstj?

@stephentoub stephentoub merged commit 672d6b0 into dotnet:master Jan 28, 2019
@stephentoub stephentoub deleted the asyncdisposablerefs branch January 28, 2019 19:25
@ericstj
Copy link
Member

ericstj commented Jan 28, 2019

We do ref -> src comparison for anything with a reference. We don't do the src -> ref comparison (AKA: reverse) for things with runtime references: https://github.com/dotnet/arcade/blob/cccfb5be4409acef7e07981d2b4fceeb1d9a992f/src/Microsoft.DotNet.ApiCompat/build/Microsoft.DotNet.ApiCompat.targets#L89-L90

The reasoning given in the initial commit was

It is skipped for projects with runtime references because those are
typically many implementation to one ref which doesn't work as well
for verifying the APIs match.

@karelz karelz added this to the 3.0 milestone Mar 18, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Apparently when I added the DisposeAsync methods to the refs, I didn't actually mark the corresponding types as implementing the interface.  This fixes that.

Commit migrated from dotnet/corefx@672d6b0
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.

4 participants