-
Notifications
You must be signed in to change notification settings - Fork 2
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
Support Roslyn 3.11, 4.4, 4.6, and 4.8 #73
Support Roslyn 3.11, 4.4, 4.6, and 4.8 #73
Conversation
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.
Borrowed this technique from xunit.analyzers - should work to resolve our multi-targeting issues for end users. The real problem isn't the version of .NET being targeted, it's the version of the Visual Studio / .NET SDK tooling being used: https://learn.microsoft.com/en-us/visualstudio/extensibility/roslyn-version-support?view=vs-2022
run: dotnet test -c Release | ||
shell: bash | ||
run: | | ||
if [ "${{ matrix.os }}" = "ubuntu-latest" ]; then |
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.
Can't run .NET Framework tests reliably on Linux; worked around that here.
<When Condition="$(MSBuildProjectName.Contains('.Tests'))"> | ||
<ItemGroup> | ||
<!-- Download packages referenced by ReferenceAssembliesHelper --> | ||
<PackageDownload Include="Akka.Cluster.Sharding" Version="[1.5.15]"/> |
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.
Moved this to here so all of the individual .Tests
projects could all download the same dependencies needed for running integration tests
@@ -21,6 +26,7 @@ | |||
<LangVersion>latest</LangVersion> | |||
<ImplicitUsings>enable</ImplicitUsings> | |||
<Nullable>enable</Nullable> | |||
<RoslynVersion>4.8.0</RoslynVersion> |
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.
The key driver of this Roslyn-versioning system is this custom property - this is used to specify the version of the Roslyn tooling that gets included inside each distributable.
@@ -18,4 +18,8 @@ | |||
<PackageVersion Include="FluentAssertions" Version="6.12.0"/> | |||
<PackageVersion Include="coverlet.collector" Version="6.0.1"/> | |||
</ItemGroup> | |||
|
|||
<ItemGroup> | |||
<GlobalPackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="all" /> |
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.
Added SourceLink as a global package reference for all projects in the solution.
</PropertyGroup> | ||
<PropertyGroup Condition="$(MSBuildProjectName.Contains('Roslyn311'))"> | ||
<DefineConstants>$(DefineConstants);ROSLYN_3_11;ROSLYN_3_11_OR_GREATER</DefineConstants> | ||
<RoslynVersion>3.11.0</RoslynVersion> |
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.
Defines the versions of Roslyn we use based on the naming convention of the projects
@@ -22,6 +22,14 @@ | |||
|
|||
<file target="analyzers\dotnet\cs\" src="..\Akka.Analyzers\bin\$Configuration$\netstandard2.0\Akka.Analyzers.dll" /> | |||
<file target="analyzers\dotnet\cs\" src="..\Akka.Analyzers.Fixes\bin\$Configuration$\netstandard2.0\Akka.Analyzers.Fixes.dll" /> | |||
<!-- <file target="tools\" src="..\xunit.analyzers.fixes\tools\*.ps1" />--> | |||
|
|||
<file target="analyzers\dotnet\roslyn3.11\cs\" src="..\Akka.Analyzers.Fixes.Roslyn311\bin\$Configuration$\netstandard2.0\Akka.Analyzers.dll" /> |
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 is what makes the solution workable for end-users - this moves compiler-platform specific versions of the Roslyn analyzers + fixes into the directories expected by the .NET Compiler platform. So if the user is running version 4.3 of Roslyn, the system should fall back to using the 4.2 version specified in this directory.
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Microsoft.CodeAnalysis"/> | ||
<PackageReference Include="Microsoft.CodeAnalysis" VersionOverride="[$(RoslynVersion)]"/> |
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 is where we specify the version of Roslyn tooling that will get used in the solution. All of the .Roslyn{X}
solutions update their RoslynVersion
variables accordingly and reference the tooling accordingly.
@@ -9,7 +9,7 @@ namespace Akka.Analyzers; | |||
/// <summary> | |||
/// INTERNAL API | |||
/// </summary> | |||
internal static class Guard | |||
public static class Guard |
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.
So I don't have to declare every compiler version as a Friend assembly.
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.
LGTM
Changes
close #66
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):