-
Notifications
You must be signed in to change notification settings - Fork 192
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
upgrade to net8.0 #395
upgrade to net8.0 #395
Conversation
@@ -1,7 +1,7 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> | |||
|
|||
<PropertyGroup> | |||
<TargetFramework>net6.0</TargetFramework> | |||
<TargetFramework>net8.0</TargetFramework> |
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 should be using <TargetFrameworks>net8.0;net7.0;net6.0;</TargetFrameworks>
instead. The package should compile to all current .NET runtimes.
@@ -1,7 +1,7 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> | |||
|
|||
<PropertyGroup> | |||
<TargetFramework>net6.0</TargetFramework> | |||
<TargetFramework>net8.0</TargetFramework> |
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 should be using <TargetFrameworks>net8.0;net7.0;net6.0;</TargetFrameworks>
instead. The package should compile to all current .NET runtimes.
@@ -1,7 +1,7 @@ | |||
<Project Sdk="Microsoft.NET.Sdk.Razor"> | |||
|
|||
<PropertyGroup> | |||
<TargetFramework>net6.0</TargetFramework> | |||
<TargetFramework>net8.0</TargetFramework> |
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 should be using <TargetFrameworks>net8.0;net7.0;net6.0;</TargetFrameworks>
instead. The package should compile to all current .NET runtimes.
@@ -20,8 +20,8 @@ | |||
</PropertyGroup> | |||
|
|||
<ItemGroup> |
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.
You're going to need to make the references conditional on the active runtime. See https://github.com/CloudNimble/BlazorEssentials/blob/main/src/CloudNimble.BlazorEssentials/CloudNimble.BlazorEssentials.csproj#L38-L66
I added Multitargetting to the library project, but left the demo and docs at net8 |
Nice! Only thing I would say from here, and this is super nit-picky I know, is to go into the .csproj and .props files and Ctrl+e, D the documents so that the indents are consistent. |
@robertmclaws Is there anything still holding up this PR? I have an interest in using a net8 version directly. Thanks! |
I think it looks great and can be merged. |
Hello, thanks for the PR! I just learned a new thing! Never knew about the props file... I will test this locally quickly and merge asap, thanks again |
No description provided.