-
Notifications
You must be signed in to change notification settings - Fork 40
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
Swim refactor #176
Swim refactor #176
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.
Looks good after the first pass. I will make another pass later. Didn't want to repeat myself too much, but from the cosmetic side of the world you can:
- use aliases (
Stream
,IO
etc.) rather than Z-fellas :) - make case classes final
@mijicd @mschuwalow work is done to the point I feel comfortable to merge it. I plan to add config for SWIM, layers for I need to look why tests are hanging on CI. I run them more than 10 times locally and I couldn't have reproduced it. Maybe this is something with UDP. |
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.
Very nice work! I've made some remarks and tried to do the annoying ones as commit suggestions.
keeper/src/main/scala/zio/keeper/membership/swim/Broadcast.scala
Outdated
Show resolved
Hide resolved
keeper/src/main/scala/zio/keeper/membership/swim/Broadcast.scala
Outdated
Show resolved
Hide resolved
keeper/src/main/scala/zio/keeper/membership/swim/Broadcast.scala
Outdated
Show resolved
Hide resolved
keeper/src/main/scala/zio/keeper/membership/swim/protocols/FailureDetection.scala
Outdated
Show resolved
Hide resolved
keeper/src/main/scala/zio/keeper/membership/swim/protocols/User.scala
Outdated
Show resolved
Hide resolved
Co-Authored-By: Dejan Mijić <dmijic@acm.org>
Co-Authored-By: Dejan Mijić <dmijic@acm.org>
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. I'll take a close look tomorrow
No description provided.