Add net5 and net6 as TargetFrameworks
#479
Conversation
oformaniuk
force-pushed
the
chore/new-targets
branch
from
ad57b3b to
e15a7f7
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| <ItemGroup Condition="'$(TargetFramework)'=='net5'"> | ||
| <PackageReference Include="Microsoft.CSharp" Version="4.7.0" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition="'$(TargetFramework)'=='net6'"> | ||
| <PackageReference Include="Microsoft.CSharp" Version="4.7.0" /> | ||
| </ItemGroup> | ||
|
|
There was a problem hiding this comment.
Microsoft.CSharp is a polyfill package hence this dependency should be able to be removed on the newer frameworks.
|
Closes #573 |
|
@oformaniuk & @StefH I see that this has been sitting around for a while, any chance the conflicts could be resolved and then this merge progressed. :) |
I'd be happy to handle merging part if you can handle conflicts :) |
| <ItemGroup Condition="'$(TargetFramework)'=='netcoreapp3.1' or '$(TargetFramework)'=='netcoreapp2.1'"> | ||
| <ItemGroup Condition="'$(TargetFramework)'=='netcoreapp3.1' or '$(TargetFramework)'=='netcoreapp2.1' or '$(TargetFramework)'=='net5' or '$(TargetFramework)'=='net6'"> |
There was a problem hiding this comment.
Condition should be simplified to improve maintenance going forward.
|
|
||
| <ItemGroup Condition="'$(ContinuousIntegrationBuild)' != 'true'"> | ||
| <PackageReference Include="SonarAnalyzer.CSharp" Version="8.33.0.40503"> | ||
| <PrivateAssets>all</PrivateAssets> | ||
| <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | ||
| </PackageReference> | ||
| </ItemGroup> |
There was a problem hiding this comment.
Should be bundled in existing item group
| <PropertyGroup Condition="'$(TargetFramework)'=='net5'"> | ||
| <DefineConstants>$(DefineConstants);netcoreapp;netstandard;net</DefineConstants> | ||
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup Condition="'$(TargetFramework)'=='net6'"> | ||
| <DefineConstants>$(DefineConstants);netcoreapp;netstandard;net</DefineConstants> | ||
| </PropertyGroup> | ||
|
|
There was a problem hiding this comment.
Should be checked if this is really needed.
|
@oformaniuk I will take a look later today to see if I can push changes to this branch. |
If not, it can be done by forking and merging back to this branch via PR 🙂 |
|
@oformaniuk pr #577 has been created to resolve the conflicts and implements the feedback i have above. |
|
@thompson-tomo and @oformaniuk Side note: do we actually want to add .NET 5 because that one is out of support for some time? And instead just add .NET 6 and .NET 8 because .NET 7 is also almost out of official support? |
|
@StefH i agree. What I will do tomorrow morning my time is cherry pick the commits on to a new branch from master & then submit that as a fresh pr & close my other pr. Also no issue on my side with only doing net 6 |
|
As agreed, change will be handled in a new PR. |
|
New PR is #578 which is a nice and clean PR. |
No description provided.