Added .NET 8 target #696

multiarc · 2025-01-12T00:37:18Z

Added .NET 8 Support
- Aligned package versions with respective targets (.NET 6, netstandard2.0, .NET 8.0)
- Removed .NET 7.0 target and outdated references
- Removed direct targets on the library for .NET Framework and only left netstandard2.0 because of dependencies, netstandard2.0 reference required anyways and there is no point to split targets on the library itself.
StyleCop is deprecated
- Removed StyleCop.Analyzers in favor of Microsoft.CodeAnalysis.NetAnalyzers (debatable, but feels appropriate in this case)
- Fixed nullable references issues and other minor issues
Moved packages for unit tests out of .props file
Upgraded xUnit
- Fixed xUnit warnings
Upgraded the rest of packages in unit tests
Added packages packages.lock.json for repeatable restore

1.1 Aligned package versions with respective targets (.NET 6, netstandard2.0, .NET 8.0) 1.2 Removed .NET 7.0 target 1.3 Removed direct targets on the library for .NET Framework and only left netstandard2.0 because of dependencies, netstandard2.0 reference required anyways and there is no point to split targets on the library itself. 2. StyleCop is deprecated 2.1 Removed StyleCop 2.2 Added Microsoft.CodeAnalysis.NetAnalyzers 2.3 Fixed nullable references issues and other minor issues 3. Moved packages for unit tests out of .props file 4. Upgraded xUnit 4.1 Fixed xUnit warnings 5. Upgraded the rest of packages in unit tests Signed-off-by: Aliaksandr Kukrash <[email protected]>

…ect settings to .props file Signed-off-by: Aliaksandr Kukrash <[email protected]>

Signed-off-by: Aliaksandr Kukrash <[email protected]>

… latest .NET version in dotnet-install.sh Signed-off-by: Aliaksandr Kukrash <[email protected]>

Turnerj

Hey - appreciate the effort you went to with this PR. I've added some comments.

Turnerj · 2025-01-14T11:04:28Z

azure-pipelines.yml

            inputs:
              packageType: "sdk"
-              useGlobalJson: true


What made you want to switch from the global.json being used to specifying the .NET 8 SDK manually? Just consistency with the others?

Ideally, you would use latest available patch when building project as opposed to whatever is laying around in your global.json, unless there is a compatibility issue. This was the reason of removal

Turnerj · 2025-01-14T11:07:34Z

Tools/Schema.NET.Tool/Schema.NET.Tool.csproj

-      <TargetPathWithTargetPlatformMoniker Include="$(PKGSystem_Threading_Tasks_Extensions)\lib\netstandard2.0\*.dll" IncludeRuntimeDependency="false" />
-    </ItemGroup>
-  </Target>
+<!--  <PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">-->


This whole section was to work around transitive dependency issues with source generators (basically if your dependencies have dependencies, you need to specify them all for source generators). Did you not have issues with this with the code compiling?

I guess my only thought is the lock files may help resolve this but I haven't seen anyone mention that on the discussions on GitHub about source generators before.

Do you have repro steps for this? I will check and add some tests as well to cover this case.

Turnerj · 2025-01-14T11:08:13Z

Tools/Schema.NET.Tool/Schema.NET.Tool.csproj

@@ -1,62 +1,42 @@
 <Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup Label="Build">
-    <TargetFrameworks>netstandard2.0</TargetFrameworks>
+    <TargetFrameworks>net8.0;net6.0;netstandard2.0</TargetFrameworks>


Source generators can only target netstandard2.0 unfortunately.

This one is interesting. Purely speaking you can use any target for source generator: after all this is just Roslyn, you have all targets available. But! It only works in console (dotnet command line) and in Rider IDE. Visual Studio and MSBuild cannot do that properly and spawns all sorts of issues (missing references, not able to compile, etc.)

As opposed to command line (view from Rider):

I can even remove netstandard2.0 completely from the targets and it continues to function no problem.

I will look into it.

I split Tool project into separate projects to allow proper package versioning for different targets when built from console (and published as a package) - as you cannot properly reference different versions of System.Text.Json and others from within single target. While allowing usage from Visual Studio and falling back to netstandard2.0 only.

Turnerj · 2025-01-14T11:10:34Z

Tests/Schema.NET.Test/Schema.NET.Test.csproj

@@ -1,13 +1,27 @@
 <Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup Label="Build">
-    <TargetFrameworks>net7.0;net6.0;net472</TargetFrameworks>
+    <TargetFrameworks>net8.0;net6.0;net462;net472;net48</TargetFrameworks>


Not sure how necessary it is to test 3 different versions of .NET Framework. I'd personally be happy to just test one .NET Framework version (likely 4.8).

I don't see a harm of listing major relevant versions in here. Benchmarks is another story and probably it worth removing. But I agree, generally speaking it shouldn't matter.

Please let me know how to proceed here. Keep all net versions or leave net48 only?

.github/workflows/build.yml

Signed-off-by: Aliaksandr Kukrash <[email protected]>

…other IDEs and dotnet command line. Signed-off-by: Aliaksandr Kukrash <[email protected]>

Signed-off-by: Aliaksandr Kukrash <[email protected]>

github-actions bot added enhancement Issues describing an enhancement or pull requests adding an enhancement. maintenance Pull requests that perform maintenance on the project but add no features or bug fixes. labels Jan 12, 2025

Explicitly enable NET Analyzers for all projects and move common proj…

516f66b

…ect settings to .props file Signed-off-by: Aliaksandr Kukrash <[email protected]>

multiarc marked this pull request as ready for review January 12, 2025 00:42

multiarc added 3 commits January 12, 2025 01:46

Test pipeline update

f7d246c

Signed-off-by: Aliaksandr Kukrash <[email protected]>

Remove extra "UseDotNet" task since latest SDK should be sufficient

43b79d4

Signed-off-by: Aliaksandr Kukrash <[email protected]>

Put latest SDK version as AppVeyor build failed as it cannot discover…

8609f37

… latest .NET version in dotnet-install.sh Signed-off-by: Aliaksandr Kukrash <[email protected]>

multiarc mentioned this pull request Jan 12, 2025

Version 12.0.0 has a dependency on .NET 7 in the .NET 6 build #598

Open

Turnerj requested changes Jan 14, 2025

View reviewed changes

multiarc marked this pull request as draft January 14, 2025 21:20

multiarc added 3 commits January 14, 2025 22:34

Restore direct references for Visual Studio, WIP

bdcf195

Signed-off-by: Aliaksandr Kukrash <[email protected]>

Fallback to netstandard2.0 in Visual Studio and allow any target for …

dbb7059

…other IDEs and dotnet command line. Signed-off-by: Aliaksandr Kukrash <[email protected]>

Merge setup-dotnet

e9214e5

Signed-off-by: Aliaksandr Kukrash <[email protected]>

multiarc force-pushed the feature/net8_support branch from 34e1c81 to e9214e5 Compare January 14, 2025 21:59

multiarc added 3 commits January 14, 2025 23:04

Move Resources.Designer.cs out of shared folder

b5abdf5

Signed-off-by: Aliaksandr Kukrash <[email protected]>

Fix root namespace

324d2fc

Signed-off-by: Aliaksandr Kukrash <[email protected]>

Rename folder to Common

86e5f17

Signed-off-by: Aliaksandr Kukrash <[email protected]>

multiarc requested a review from Turnerj January 14, 2025 23:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added .NET 8 target #696

Added .NET 8 target #696

multiarc commented Jan 12, 2025 •

edited

Loading

Turnerj left a comment

Turnerj Jan 14, 2025

multiarc Jan 14, 2025

Turnerj Jan 14, 2025

multiarc Jan 14, 2025

Turnerj Jan 14, 2025

multiarc Jan 14, 2025

multiarc Jan 14, 2025

multiarc Jan 14, 2025

multiarc Jan 14, 2025 •

edited

Loading

Turnerj Jan 14, 2025

multiarc Jan 14, 2025 •

edited

Loading

multiarc Jan 14, 2025

Added .NET 8 target #696

Are you sure you want to change the base?

Added .NET 8 target #696

Conversation

multiarc commented Jan 12, 2025 • edited Loading

Turnerj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

multiarc Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

multiarc Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

multiarc commented Jan 12, 2025 •

edited

Loading

multiarc Jan 14, 2025 •

edited

Loading

multiarc Jan 14, 2025 •

edited

Loading