-
Notifications
You must be signed in to change notification settings - Fork 63
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
Added advance-message-interval option and more federated tests for TypeScript #1293
Conversation
…, as it now supports advance-message-interval.
NOTE 1: I think this change shouldn't cause any merge conflict on the fed-gen PR: #1221 NOTE 2: This is a bug fix for reactor-ts runtime |
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!
targetConfig.coordinationOptions.advance_message_interval != null) { | ||
targetConfigurations.add( | ||
"this.setAdvanceMessageInterval(${timeInTargetLanguage(targetConfig.coordinationOptions.advance_message_interval)})") | ||
} |
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 think there should be a warning here if advance_message_interval is not set by the user. At least this is how this scenario is handled in C and Python.
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.
Thank you for the suggestion! I added a warning here: 1c6a598
…t for reactor but advance message interval is not set.
Relevant reactor-ts PR: lf-lang/reactor-ts#109