-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove IsPreRelease msbuild property #72169
Conversation
The property was added in dotnet/coreclr@809b8f7 years ago and isn't used anymore. Remove it to ease branding.
Tagging subscribers to this area: @dotnet/runtime-infrastructure Issue DetailsThe property was added in dotnet/coreclr@809b8f7 years ago and isn't used anymore. Remove it to ease branding.
|
eng/native/configureplatform.cmake
Outdated
# If set, indicates that this is not an officially supported release | ||
# Keep in sync with IsPrerelease in Directory.Build.props | ||
# If set, indicates that this is not an officially supported release. | ||
# Release branches should set this to false. | ||
set(PRERELEASE 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be set to 1 in main?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was recently switched to 0 by @carlossanlop in 40b3c36. I already reached out to him offline and explained that this doesn't need to be set before we actually release.
Do you think we should switch it back and make sure that it again gets changed to 0 in the release/7.0 branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we should switch it back and make sure that it again gets changed to 0 in the release/7.0 branch?
Yes.
If we switch it to 0 in main, we will see C/C++ compiler warnings creep in. I do not think anybody wants to sign up for fixing them once we fork for .NET 8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
FWIW, it was also used by the Utf8String prototype a couple years after that and maybe others. But we do have better avenues to do this kind of things now with experimental features and runtimelab and deleting makes sense. |
The property was added in dotnet/coreclr@809b8f7 years ago and isn't used anymore. Remove it to ease branding.