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 ClusterConfiguration to legacy #3901
Changes from all commits
4861e76
de51419
235079d
71c633c
57e7de2
d25c244
26a353a
fca4397
db4cfc0
7cf0d1c
600236f
8f6a020
37a5d8e
f0d7eb8
3d6a759
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<PackageId>Microsoft.Orleans.Clustering.DynamoDB</PackageId> | ||
<Title>Microsoft Orleans clustering provider for AWS DynamoDB</Title> | ||
|
@@ -22,6 +22,7 @@ | |
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\..\Orleans.Core.Legacy\Orleans.Core.Legacy.csproj" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you need dependency on Orleans.Core.Legacy? you used reflection to get There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've pulled all references of Cluster config out of the core packages, but not all of the extension libraries. So any library that still references cluster config needs depend on core legacy. This can (and IMHO should) be handled in smaller, more targeted change sets rather than adding to this one. |
||
<ProjectReference Include="..\..\Orleans.Runtime.Abstractions\Orleans.Runtime.Abstractions.csproj" /> | ||
<ProjectReference Include="..\..\OrleansProviders\OrleansProviders.csproj" /> | ||
</ItemGroup> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,7 +34,9 @@ | |
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\..\Orleans.Core.Legacy\Orleans.Core.Legacy.csproj" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same answer. :/ |
||
<ProjectReference Include="..\..\Orleans.Runtime.Abstractions\Orleans.Runtime.Abstractions.csproj" /> | ||
<ProjectReference Include="..\..\Orleans.Runtime.Legacy\Orleans.Runtime.Legacy.csproj" /> | ||
<ProjectReference Include="..\..\OrleansProviders\OrleansProviders.csproj" /> | ||
</ItemGroup> | ||
|
||
|
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 though you went with the option of moving these legacy Configurator to Orleans.Core.Legacy package? and use reflection to configure membership and gateway list provider
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 touched on this in the PR comments. The pattern we'd planned on didn't work. The issue was that either way something needs to know about something it has no knowledge of, either the options or the clusterConfiguration. As you can see from how this pattern turned out, it's not a bad approach. Probably the least bad of the possible bad approaches.
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.
sorry my bad, I went read it and deleted some noisy comments