Expose some threading types and members - #11229
Conversation
|
@dotnet-bot test this please |
| "System.ObjectModel": "4.0.13-beta-devapi-24424-01", | ||
| "System.Reflection": "4.1.1-beta-devapi-24424-01", | ||
| "System.Reflection.Emit": "4.0.2-beta-devapi-24424-01", | ||
| "System.Runtime": "4.2.0-beta-devapi-24424-01", |
There was a problem hiding this comment.
I don't think you need this given that the test csproj should have a .pkgproj reference to System.Runtime already
|
A couple clean-up notes but otherwise looks good. |
| { | ||
| public partial class CriticalHandle | ||
| { | ||
| // Manually added because CriticalFinalizerObject.Finalize is removed. |
There was a problem hiding this comment.
Since you're adding back CriticalFinalizerObject, will this be removed? (I guess in a subsequent PR?)
There was a problem hiding this comment.
Missed this, I have added it now to issue #11231, will take care of it as part of that.
|
Some minor questions/comments, but LGTM. Thanks, @kouvel. |
|
Also added netstandard1.7 to the System.Runtime.Handles test project.json |
|
@dotnet-bot test this please |
just as FYI, @kouvel the ns1.7 tests won't run just yet as part of the CI, I'm super close to merging my changes, so you might want to hold off your change for a few hours so that your ns1.7 tests actually run before merging |
|
@joperezr, sure, I can wait. I've run them locally successfully so not expecting any issues. |
|
@kouvel for future reference it is best for reviewers if you just make new commits instead of squashing to one commit for all tweaks. That makes reviewing the deltas much easier and you can just squash before merge. |
| <AllowUnsafeBlocks>true</AllowUnsafeBlocks> | ||
| <NoWarn>1718</NoWarn> | ||
| <NugetTargetMoniker Condition="'$(TargetGroup)'=='netstandard1.7' AND '$(NugetTargetMoniker)'==''">.NETStandard,Version=v1.7</NugetTargetMoniker> | ||
| <NugetTargetMoniker Condition="'$(TargetGroup)'=='netstandard1.7'">.NETStandard,Version=v1.7</NugetTargetMoniker> |
There was a problem hiding this comment.
minor: This property is already set in dir.props (https://github.com/dotnet/corefx/blob/dev/api/dir.props#L363), based on the TargetGroup.
|
LGTM |
|
@dotnet-bot test this please |
|
I'm about to merge #11366 , can we hold this change till after that's done? |
|
@dotnet-bot test this please |
|
@kouvel looks like your build issues are all ApiCompat related. I suppose that this is happening in the uap10.1 build, which in that case you can just create a baseline for those issues and then log an issue to remove the baseline once the implementation is available on the .NetNative Targeting pack |
|
@dotnet-bot test this please |
1 similar comment
|
@dotnet-bot test this please |
|
Thanks @joperezr, I have added the compat issues to the uap101aot baseline and filed #11575 to track their removal. |
|
@dotnet-bot test this please |
1 similar comment
|
@dotnet-bot test this please |
|
@kouvel looks like you have to rebase before you test |
|
Was just wondering why this test failure is occurring only on my PR. I'll wait for the bot update to the package numbers and will merge again. |
|
Oh there was an update earlier this morning I guess |
Fixes #10933: - Exposed some members of WaitHandle - Moved SafeHandle and SafeWaitHandle from System.Runtime.Handles to System.Runtime, due to dependency from WaitHandle - Exposed some types derived from SafeHandle, and changed SafeWaitHandle's base class - Exposed some deserialization constructors - Moved and added tests as appropriate
Expose some threading types and members Fixes dotnet/corefx#10933: - Exposed some members of WaitHandle - Moved SafeHandle and SafeWaitHandle from System.Runtime.Handles to System.Runtime, due to dependency from WaitHandle - Exposed some types derived from SafeHandle, and changed SafeWaitHandle's base class - Exposed some deserialization constructors - Moved and added tests as appropriate Commit migrated from dotnet/corefx@d419292
7 participants
Fixes #10933: