This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
Put default interfaces behind a define #15358
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is needed so that we can turn default interfaces off in release branches. I can't find a central location where the PRERELEASE flag could be defined because native and managed builds seem to be completely disconnected. To limit the risk of only flipping the flag in one build type, I'm adding a test that verifies being able to load an interface with default methods matches what RuntimeFeature says.
|
@jkotas @weshaggard please take a look |
|
Cc @sergiy-k |
weshaggard
reviewed
Dec 4, 2017
dir.props
Outdated
| @@ -140,6 +140,10 @@ | |||
| <PackageThirdPartyNoticesFile>$(ProjectDir)THIRD-PARTY-NOTICES.TXT</PackageThirdPartyNoticesFile> | |||
| <SyncInfoDirectory>$(BaseIntermediateOutputPath)</SyncInfoDirectory> | |||
|
|
|||
| <!-- If true, indicates that this is not an officially supported release --> | |||
| <!-- It is important to flip this to false in official release branches --> | |||
| <IsPrerelease>true</IsPrerelease> | |||
There was a problem hiding this comment.
So we need to flip both this bit and the one in clrdefinitions.cmake? Is there a good way too hook the two up together? If not can you please add a comment in both places pointing at the other so that they should be flipped together.
There was a problem hiding this comment.
I agree. Please at least add a comment.
There was a problem hiding this comment.
Is there a good way too hook the two up together?
I couldn't find one, but I don't know much about CoreCLR build. Is there someone who's a CoreCLR build subject matter expert who could tell us?
There was a problem hiding this comment.
There is not good way to link these together. The managed and unmanaged builds have to be kept in sync manually.
sergiy-k
approved these changes
Dec 4, 2017
MichalStrehovsky
added a commit
to MichalStrehovsky/corefx
that referenced
this pull request
Dec 15, 2017
The flag indicates that the runtime supports default implementation of interfaces. Since this is a prerelease feature that is not complete yet (and we don't want to accidentally ship it), the flag is keyed off a newly added `IsPrerelease` property. This property is expected to be flipped to `false` in release branches. This corresponds to dotnet/coreclr#15358.
MichalStrehovsky
added a commit
to MichalStrehovsky/corefx
that referenced
this pull request
Dec 15, 2017
The flag indicates that the runtime supports default implementation of interfaces. Since this is a prerelease feature that is not complete yet (and we don't want to accidentally ship it), the flag is keyed off a newly added `IsPrerelease` property. This property is expected to be flipped to `false` in release branches. This corresponds to dotnet/coreclr#15358.
MichalStrehovsky
added a commit
to dotnet/corefx
that referenced
this pull request
Dec 16, 2017
The flag indicates that the runtime supports default implementation of interfaces. Since this is a prerelease feature that is not complete yet (and we don't want to accidentally ship it), the flag is keyed off a newly added `IsPrerelease` property. This property is expected to be flipped to `false` in release branches. This corresponds to dotnet/coreclr#15358.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is needed so that we can turn default interfaces off in release branches.
I can't find a central location where the PRERELEASE flag could be defined because native and managed builds seem to be completely disconnected.
To limit the risk of only flipping the flag in one build type, I'm adding a test that verifies being able to load an interface with default methods matches what RuntimeFeature says.