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
Move to using Build Props file and central package management. #333
Conversation
@@ -131,7 +131,7 @@ public void JournalOptionsTest() | |||
[Fact(DisplayName = "MongoDbJournalOptions should be bindable to IConfiguration")] | |||
public void JournalOptionsIConfigurationBindingTest() | |||
{ | |||
var stream = new MemoryStream(Encoding.UTF8.GetBytes(Json)); | |||
using var stream = new MemoryStream(Encoding.UTF8.GetBytes(Json)); |
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.
Now all projects use the same langversion can clean up old items from the tests
@@ -12,22 +12,19 @@ | |||
<GenerateDocumentationFile>true</GenerateDocumentationFile> | |||
<Description>Akka Persistence journal and snapshot store backed by MongoDB database.</Description> | |||
<NoWarn>$(NoWarn);CS1591</NoWarn> | |||
<LangVersion>latest</LangVersion> |
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.
Most projects seem to be using latest. Can change this to what ever is appropriate.
</PropertyGroup> | ||
<PropertyGroup> | ||
<NetCoreTestVersion>netcoreapp3.1</NetCoreTestVersion> | ||
<NetTestVersion>net6.0</NetTestVersion> | ||
<NetFrameworkTestVersion>net471</NetFrameworkTestVersion> | ||
<NetStandardLibVersion>netstandard2.0</NetStandardLibVersion> | ||
|
||
<AkkaVersion>1.5.8</AkkaVersion> |
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.
I have seen some projects have two variables for Akka and Akka Hosting. Not sure if required
https://github.com/akkadotnet/Akka.Persistence.Sql/blob/8052a17c75a678ca8052923a3d8ee43a19fb2bd7/src/Directory.Build.props#L24C1-L24C53
</PropertyGroup> | ||
<!-- SourceLink support for all Akka.NET projects --> | ||
<ItemGroup> | ||
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" /> | ||
<PackageReference Include="Microsoft.SourceLink.GitHub" 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.
Note I have seen some other repos that have moved to central package management have the Microsoft.SourceLink.GitHub
in the Directory.Packages.props
file but no reference either in the csproj or Directory.Build.props
I have noticed it in Akka.Persistence.Sql
and Akka.Persistence.PostgreSql
repos
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
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
I am unsure if the build system needs some things to be upgraded as well or not. Similar to