Skip to content
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

Recognize .NET 7 when installed #1194

Merged
merged 1 commit into from
Jun 29, 2022
Merged

Recognize .NET 7 when installed #1194

merged 1 commit into from
Jun 29, 2022

Conversation

CharliePoole
Copy link
Collaborator

@CharliePoole CharliePoole commented Jun 29, 2022

Fixes #1193

@CharliePoole CharliePoole merged commit abd77f8 into release-3.15.1 Jun 29, 2022
@CharliePoole CharliePoole deleted the issue-1193 branch June 29, 2022 04:29
@@ -201,6 +201,8 @@ private Version GetClrVersionForFramework(Version frameworkVersion)
return new Version(5, 0, 1);
case 6:
return new Version(6, 0, 0);
case 7:
Copy link
Member

Choose a reason for hiding this comment

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

question, out of curiosity: With the increased release cadence of new versions now compared to the netfx days, is there a benefit (or harm?) of adding a default case like return new Version(frameworkversion.Major, 0, 0)

Copy link
Collaborator Author

@CharliePoole CharliePoole Jun 29, 2022

Choose a reason for hiding this comment

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

@stevenaw Yes, but not worth the effort right here as this method doesn't exist in the 4.0 builds. Once released, 3.15.1 will become my new "final" 3.x release - unless another critical bug shows up! I'll take a look to see if we need something similar in 4.0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have verified that this problem doesn't arise at all in the 4.0 build, which contains no code similar to what I fixed here. The 4.0.0-devxxxxx builds run fine with .NET 7.0 preview 5 installed.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @CharliePoole I completely agree and understand about "not right now" 🙂

I'm mostly asking because I've seen a few places where we have this hard-coded version mapping and I wasn't sure if there'd been discussion about it yet, or considered and ruled out for some reason. I didn't realize it had already been adjusted in the 4.0 build.

@CharliePoole CharliePoole restored the issue-1193 branch June 30, 2022 14:11
@CharliePoole CharliePoole deleted the issue-1193 branch January 1, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants