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

Roll back default language version from 11 to 10 #799

Merged
merged 6 commits into from May 4, 2023

Conversation

adrianwyatt
Copy link
Contributor

@adrianwyatt adrianwyatt commented May 4, 2023

Motivation and Context

C# 11 requires .NET 7+ and we would like to keep the requirement at .NET 6 for now.

  • includes a minor cosmetic cleanup for the user-agent name in example 21
  • removed unused common.props

@adrianwyatt adrianwyatt requested a review from shawncal May 4, 2023 00:23
@adrianwyatt adrianwyatt enabled auto-merge (squash) May 4, 2023 00:23
@adrianwyatt adrianwyatt self-assigned this May 4, 2023
@adrianwyatt adrianwyatt added bug Something isn't working .NET Issue or Pull requests regarding .NET code PR: ready to merge PR has been approved by all reviewers, and is ready to merge. kernel Issues or pull requests impacting the core kernel labels May 4, 2023
@stephentoub
Copy link
Member

C# 11 requires .NET 7+ and we would like to keep the requirement at .NET 6 for now.

The .NET 7 SDK. You can still run on .NET 6. Why do you need to stay on the .NET 6 SDK?

@adrianwyatt adrianwyatt disabled auto-merge May 4, 2023 00:37
@shawncal
Copy link
Member

shawncal commented May 4, 2023

C# 11 requires .NET 7+ and we would like to keep the requirement at .NET 6 for now.

The .NET 7 SDK. You can still run on .NET 6. Why do you need to stay on the .NET 6 SDK?

I think this is because we have all these tutorials for sample apps that say to clone the repo, build a few projects, and go. Those projects aren't all .net 7 yet (we were trying to maximize compatibility). The long-term fix here is to ensure all those samples are pulling from our nuget feed and not using project references (...but then they also don't automatically inherit our refactoring changes like renaming things everywhere), or to just update all the samples to 7.0 and make that a requirement in the getting started docs.

I'm ok with a temporary reversion here while the SDK is still under development and names are changing. I'll capture a task to update samples and update SDK lang version.

@stephentoub Does that sound okay?

@adrianwyatt
Copy link
Contributor Author

C# 11 requires .NET 7+ and we would like to keep the requirement at .NET 6 for now.

The .NET 7 SDK. You can still run on .NET 6. Why do you need to stay on the .NET 6 SDK?

We will need to update documentation and other assets to state having .NET 6 as a requirement. I don't think we are averse to it, we just want to be intentional about it.

@shawncal
Copy link
Member

shawncal commented May 4, 2023

@adrianwyatt Am I right about the reason for the reversion above? Where did this surface?

@stephentoub
Copy link
Member

Does that sound okay?

Yup. You get to decide what .NET SDK you require, and I can understand wanting to stick with .NET 6 for now (e.g. I could imagine a valid answer being "we want to advertise that we only require an LTS version"). I just want to make sure we're encouraging staying relatively up-to-date, and there are nice features in C# 11 for this domain, e.g. raw string literals.

@adrianwyatt
Copy link
Contributor Author

@adrianwyatt Am I right about the reason for the reversion above? Where did this surface?

Yup!

@adrianwyatt adrianwyatt enabled auto-merge (squash) May 4, 2023 00:42
@adrianwyatt adrianwyatt disabled auto-merge May 4, 2023 00:42
@adrianwyatt adrianwyatt merged commit 0651d20 into microsoft:main May 4, 2023
11 checks passed
@adrianwyatt adrianwyatt deleted the rollback-langver10 branch May 10, 2023 17:41
codebrain pushed a commit to searchpioneer/semantic-kernel that referenced this pull request May 16, 2023
### Motivation and Context
C# 11 requires .NET 7+ and we would like to keep the requirement at .NET
6 for now.

- includes a minor cosmetic cleanup for the user-agent name in example
21
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
### Motivation and Context
C# 11 requires .NET 7+ and we would like to keep the requirement at .NET
6 for now.

- includes a minor cosmetic cleanup for the user-agent name in example
21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: ready to merge PR has been approved by all reviewers, and is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants