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
Add support for new update types #446
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.
This looks great.
@@ -21,3 +22,11 @@ export const LOG_STREAM_MESSAGE = { | |||
ERROR: 'ERROR', | |||
INCOMPLETE: 'INCOMPLETE' | |||
}; | |||
|
|||
export const STATE_UPDATE_TYPE = { | |||
complete_state: 'COMPLETE', |
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.
nit: I prefer the consistency of the string being the same as the key, unless there is a reason to not do this.
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.
It seemed strange if snapshot is semantically closer to incremental, yet in nearly all the test cases the change was snapshot => complete_state. The semantic differences have to do with the TIME_WINDOW in the log synchronizer, so i realized that if we just have a single sample then there is no semantic difference.
I think we need some additional tests on the logSynchronizer independent of this, which you may already ahve in the works.
No description provided.