-
Notifications
You must be signed in to change notification settings - Fork 37
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
Ensure StateChange are posted in order to Handler #15
Conversation
This is another approach for solving issue #11 that was addressed with PR #12. The advantage of this approach over #12 is that there will be atmost 1 go-routine per type of event (StateChange and error). Added a test that demonstrates that not receiving from state change and error channels do not block the Node. Also fixed a flapping test. Resolves #11
@@ -7,7 +7,9 @@ import ( | |||
"testing" | |||
"time" | |||
|
|||
"fmt" |
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.
sort these, all pkg ones at top and 3rd party separated by line space and at the bottom.
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 about that. The IDE is integrated with go fmt, etc.. and usually orders correctly those. Not sure what happened.
n.stateChg = append(n.stateChg, sc) | ||
// Invoke postStateChange only for the first state change added. | ||
// Check postStateChange for details. | ||
if len(n.stateChg) == 1 { |
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.
Can this change again before you check the length?
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.
No it can't, we are under the node's lock when adding, checking and removing.
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.
ok!
This is another approach for solving issue #11 that was addressed
with PR #12. The advantage of this approach over #12 is that there
will be atmost 1 go-routine per type of event (StateChange and error).
Added a test that demonstrates that not receiving from state change
and error channels do not block the Node.
Also fixed a flapping test.
Resolves #11