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

Conversation

@ghost
Copy link

@ghost ghost commented May 22, 2017

This api was approved here:

Fix https://github.com/dotnet/corefx/issues/5884

and is a necessary step to fixing the System.Dynamic.Runtime.Tests
failure:

https://github.com/dotnet/corefx/issues/19895

which is caused by Microsoft.CSharp trying to do the impossible
and emulate this api without GetMetadataToken() support.

CoreCLR implementation here:

dotnet/coreclr#11774

and CoreRT implementation in CR:

@ghost ghost added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label May 22, 2017
@ghost
Copy link
Author

ghost commented May 22, 2017

Cannot merge until dotnet/coreclr#11774 propagates to corefx (and preferably, corert implementation checked in.)

@ghost ghost requested review from MichalStrehovsky and jkotas May 22, 2017 17:27
@karelz karelz assigned ghost May 23, 2017
@karelz karelz added this to the 2.1.0 milestone May 23, 2017
@ghost ghost removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label May 30, 2017
@ghost
Copy link
Author

ghost commented May 30, 2017

Finally, CoreCLR was updated...

@ghost
Copy link
Author

ghost commented May 30, 2017

@dotnet-bot test Linux x64 Tests - Release - Debian.87.Amd64.Open

2 similar comments
@ghost
Copy link
Author

ghost commented May 30, 2017

@dotnet-bot test Linux x64 Tests - Release - Debian.87.Amd64.Open

@ghost
Copy link
Author

ghost commented May 31, 2017

@dotnet-bot test Linux x64 Tests - Release - Debian.87.Amd64.Open

@ghost ghost requested a review from VSadov May 31, 2017 13:58
@karelz
Copy link
Member

karelz commented Jun 11, 2017

@atsushikan what's the status of this PR? 11 days no update ...

@ghost ghost requested a review from stephentoub June 12, 2017 13:36
@ghost
Copy link
Author

ghost commented Jun 12, 2017

what's the status of this PR?

What it needs it is somebody to approve it.

@ghost
Copy link
Author

ghost commented Jun 12, 2017

@dotnet-bot test Portable Windows x64 Release Build

@karelz
Copy link
Member

karelz commented Jun 12, 2017

What it needs it is somebody to approve it.

OK, are you driving that? BTW: That's why the PR is assigned to you, the (MS) author - you are expected to take action, get code reviews, chase down people, etc.

Copy link
Member

@VSadov VSadov left a comment

Choose a reason for hiding this comment

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

LGTM
In particular since this is just exposing emplementation from dotnet/coreclr#11774 and adding tests.

@ghost ghost merged commit eb70941 into dotnet:master Jun 12, 2017
@ghost ghost deleted the sameas branch June 12, 2017 23:51
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Portable mechanism needed to determine if two MemberInfos are backed by the same metadata definition

3 participants