-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Akka.Cluster.Tools: deprecate ClustersSingletonManagerSettings.ConsiderAppVersion #7302
Akka.Cluster.Tools: deprecate ClustersSingletonManagerSettings.ConsiderAppVersion #7302
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.
Detailed my changes
if (settings.ConsiderAppVersion) | ||
#pragma warning restore CS0618 // Type or member is obsolete | ||
{ | ||
Log.Warning("As of Akka.NET v1.5.27, The 'ConsiderAppVersion' setting is no longer supported and will " + |
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.
Add a warning for users who had ConsiderAppVersion
enabled - it was disabled by default, so it shouldn't affect too many people.
@@ -188,7 +189,9 @@ private static string RoleOption(string role) | |||
RemovalMargin = removalMargin; | |||
HandOverRetryInterval = handOverRetryInterval; | |||
LeaseSettings = leaseSettings; | |||
#pragma warning disable CS0618 // Type or member is obsolete | |||
ConsiderAppVersion = considerAppVersion; |
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.
Mark the setting as obsolete - it's already not getting used.
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 rest of the code changes in this file are fixing a few linger nullability issues and cleaning up TBD
XML-DOC comments.
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
A remember-entities spec failed - I'll have to dig into that one later. |
Changes
Cleanup from PR #7298
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
AppVersion
joins cluster #7197