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

Add support for net9.0 #16573

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

Add support for net9.0 #16573

wants to merge 33 commits into from

Conversation

sebastienros
Copy link
Member

@sebastienros sebastienros commented Aug 15, 2024

Keeping track of all the tasks when adding/removing a TFM (please update so we can document it):

  • Update global.json to the require SDK version.
  • Updated src/OrchardCore.Build/TargetFrameworks.props.
  • Add custom AspNetCorePackagesVersion for each TFM in Directory.Packages.props
  • Update all uses: actions/setup-dotnet@v4 tasks to the required SDK version.
  • Update all dotnet publish, dotnet build and dotnet test calls to the latest TFM, if specified.
  • Update all tasks.json files to target the latest TFM
  • Update templates choices (see the template.json files).
  • Update docker file base images.
  • Update documentation pages specifying a TFM (search for <TargetFramework>).
  • Add a note about the supported .NET versions to the upcoming release notes.

@hishamco
Copy link
Member

Is this will be a part of OC 2.0.0 or beyond?

@sebastienros
Copy link
Member Author

@hishamco I hope we ship 2.0 before dotnet ships .NET 9

@sebastienros
Copy link
Member Author

There are a few changes we might want to take in 2.0 instead of this PR. Mostly all the code changes (formatting and obsolete method).

@MikeAlhayek
Copy link
Member

I think we should at least run the build and test the pipeline with .net 9 just to make sure we don't have anything to report.

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

Copy link
Contributor

github-actions bot commented Sep 1, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

@MikeAlhayek
Copy link
Member

@hishamco i suggest that you don't merge main on PR others are constantly working on. Let them do that if they choose to do it "when they are ready for it"

@hishamco
Copy link
Member

hishamco commented Sep 1, 2024

I just do it when there's a conflict

@MikeAlhayek
Copy link
Member

I suggest you leave that work for the PR author. Let them work on their own code and resolve their own conflict.

@hishamco
Copy link
Member

hishamco commented Sep 1, 2024

It's not big, this could happen occasionally, see #4466. I don't know what's the issue if you want anyone update branch in your PRs let me know :)

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

@MikeAlhayek
Copy link
Member

resolved conflicts

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

@sebastienros
Copy link
Member Author

@agriffard doing a local dotnet build will show all the errors so you can fix them all at the same time.

@agriffard
Copy link
Member

@agriffard doing a local dotnet build will show all the errors so you can fix them all at the same time.

Ok, thank you, the last change fixed the build.

Copy link
Contributor

github-actions bot commented Nov 9, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

@agriffard
Copy link
Member

Directory.Packages.props contains this:

<!--
  Important: the version of the Microsoft.IdentityModel.Protocols.OpenIdConnect package MUST
  match the IdentityModel version transitively referenced by OpenIddict to ensure we don't
  accidentally end up referencing inconsistent versions (which is not supported by IM).
 
  See https://github.com/OrchardCMS/OrchardCore/pull/16057 for more information.
-->
<PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.1.0" />

but https://www.nuget.org/packages/Microsoft.IdentityModel.Protocols.OpenIdConnect/8.1.0 has been unlisted by the owner

@agriffard
Copy link
Member

Main merged and build fixed again.

What changed exactly that makes the build fail on warnings like formatting or remaining unnecessary using?

@Piedone
Copy link
Member

Piedone commented Nov 9, 2024

Static code analyzers are also updated with .NET, so they can warn about previously unnoticed issues.

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants