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.

Sign up for GitHub

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

Jump to bottom

added Akka.Analyzer reference #7031

Merged
merged 6 commits into from Jan 3, 2024
Merged

added Akka.Analyzer reference #7031

merged 6 commits into from Jan 3, 2024

Conversation

Aaronontheweb
Copy link
Member

@Aaronontheweb Aaronontheweb commented Jan 2, 2024 •
edited

Changes

Adding https://github.com/akkadotnet/akka.analyzers support into Akka.NET itself.

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

@Aaronontheweb Aaronontheweb changed the title added Akka.Analyzer reference; bumped Msft.Ext minVer to 7.0 added Akka.Analyzer reference Jan 3, 2024
@Aaronontheweb
Copy link
Member Author

Was able to roll the changes back to the MSFT.EXT version range - turns out I had to upgrade to .NET 7.0 originally only because I have screwed up the NuGet package format for the Roslyn Analyzers themselves. Fixed that in https://github.com/akkadotnet/akka.analyzers/releases/tag/0.1.1 and was able to roll it back.

@Aaronontheweb Aaronontheweb added the rosyln-analyzer Issues that should be addressed via user-exposed Roslyn analyzers. label Jan 3, 2024
@Aaronontheweb Aaronontheweb added this to the 1.5.15 milestone Jan 3, 2024
Aaronontheweb
Aaronontheweb commented Jan 3, 2024
View reviewed changes
Copy link
Member Author

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detailed my changes - one last thing we need to do is to ensure that the core Akka package lists this analyzer package as a dependency. That will allow Akka.Analyzers to be consumed from end-user applications automatically, which is the entire point. This PR doesn't do that. I think I'll handle that one in a separate PR later.

src/Akka.sln
@@ -56,6 +56,8 @@ EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Build", "Build", "{10C5B1E8-15B5-4EB3-81AE-1FC054FE1305}"
ProjectSection(SolutionItems) = preProject
..\build.fsx = ..\build.fsx
Directory.Build.props = Directory.Build.props
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added these to solution file so they would be easier to edit.

src/Directory.Build.props
@@ -69,6 +69,9 @@ If you want to see the [full set of changes made in Akka.NET v1.5.14, click here
| 1 | 227 | 25 | Drew |
| 1 | 1 | 1 | szaliszali |</PackageReleaseNotes>
</PropertyGroup>
<ItemGroup Label="Analyzers">
<PackageReference Include="Akka.Analyzers" Version="0.1.2" PrivateAssets="all" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Included this package as a private asset in Directory.Build.props to the analysis would be applied solution-wide.

src/core/Akka.Cluster/Routing/ClusterRoutingConfig.cs
@@ -298,7 +298,9 @@ public override int GetNrOfInstances(ActorSystem system)
/// <inheritdoc cref="RouterConfig.CreateRouterActor"/>
public override ActorBase CreateRouterActor()
{
#pragma warning disable AK1000
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabling AK1000 here because custom router types kind of fall outside the normal use cases for not new-ing up actors

src/core/Akka.Cluster/Routing/ClusterRoutingConfig.cs
@@ -298,7 +298,9 @@ public override int GetNrOfInstances(ActorSystem system)
/// <inheritdoc cref="RouterConfig.CreateRouterActor"/>
public override ActorBase CreateRouterActor()
{
#pragma warning disable AK1000
return new ClusterRouterPoolActor(Local.SupervisorStrategy, Settings);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabling AK1000 here because custom router types kind of fall outside the normal use cases for not new-ing up actors

src/core/Akka.Docs.Tests/Streams/StreamRefsDocTests.cs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying AK1001 fixes here (PipeTo closures)

src/core/Akka.Tests/Actor/ActorRefSpec.cs
@@ -179,7 +179,7 @@ public async Task An_ActorRef_should_support_nested_ActorOfs()
var a = Sys.ActorOf(Props.Create(() => new NestingActor(Sys)));
a.Should().NotBeNull();

Func<Task> t1 = async () =>
var t1 = async () =>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated fix

src/core/Akka.TestKit/Akka.TestKit.csproj
@@ -29,7 +29,7 @@
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="$(MicrosoftLibVersion)" />
<PackageReference Include="Nito.AsyncEx.Coordination" Version="5.1.2" />
<PackageReference Include="Nito.AsyncEx.Context" Version="5.1.2" />
<PackageReference Include="System.Linq.Async" Version="$(MicrosoftLibVersion)" />
<PackageReference Include="System.Linq.Async" Version="6.0.1" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System.Linq.Async is actually part of Reactive Extensions (Rx) so it doesn't ship at the normal / same cadences as the rest of the MSFT libraries, so we shouldn't include it in the version range.

src/core/Akka.Tests/Dispatch/AsyncAwaitSpec.cs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying AK1001 fixes here (PipeTo closures)

src/core/Akka.Tests/Routing/RoutingSpec.cs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying AK1001 fixes here (PipeTo closures)

src/examples/Cluster/Metrics/Samples.Cluster.Metrics.Common/FactorialBackend.cs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying AK1001 fixes here (PipeTo closures)

@Aaronontheweb
Copy link
Member Author

And as far as website documentation goes: that is planned and will happen in subsequent PRs akkadotnet/akka.analyzers#6

Arkatufus
Arkatufus approved these changes Jan 3, 2024
View reviewed changes
Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb Aaronontheweb merged commit 1a06ec1 into akkadotnet:dev Jan 3, 2024
8 of 12 checks passed
@Aaronontheweb Aaronontheweb deleted the add-akka-analyzers branch January 3, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Arkatufus Arkatufus Arkatufus approved these changes

Assignees
No one assigned
Labels
rosyln-analyzer Issues that should be addressed via user-exposed Roslyn analyzers.
Projects
None yet
Milestone
1.5.15
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Aaronontheweb @Arkatufus