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
Changed cluster client api to use IEnumerable for InitialContacts. #2602
Conversation
Will need probably api approval. But i'm going to wait after review. |
|
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 could take it or leave it with this PR. I'm not bothered by exposing System.Collections.Immutable in the constructor one way or another.
@@ -113,7 +113,7 @@ public static ClusterClientSettings Create(Config config) | |||
/// <param name="reconnectTimeout">TBD</param> | |||
/// <exception cref="ArgumentException">TBD</exception> | |||
public ClusterClientSettings( | |||
IImmutableSet<ActorPath> initialContacts, | |||
IEnumerable<ActorPath> initialContacts, |
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.
Looks good to me.
@@ -69,15 +70,13 @@ protected ClusterClientStartSpec(ClusterClientStartSpecConfig config) : base(con | |||
|
|||
protected override int InitialParticipantsValueFactory => 3; | |||
|
|||
private ImmutableHashSet<ActorPath> InitialContacts | |||
private IEnumerable<ActorPath> InitialContacts | |||
{ | |||
get | |||
{ | |||
//return new List<ActorPath>().ToImmutableHashSet(); |
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.
Delete this.
- Updated usage patterns in tests and examples
@Danthar We can't merge this Pull Request anymore, because Cluster Tools become stable. Just make another constructor and overloaded method |
I was going to update this PR. But I changed my mind. Adding a constructor overload defeats the purpose of this PR. |
And updated usage patterns in tests and examples