-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
change output buffer to setTimeout instead of setInterval #1123
Conversation
I proposed a few changes here. What do you think? |
Great refactor, all LGTM |
@fstasi, please review. Thank you! |
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
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.
note for traceability: changes in e5f922a were proposed/implemented by @kittaakos, then accepted and copied here by @davegarthsimpson.
The PR looks good to me. Please proceed with the merge if this is ready. Thank you! |
Motivation
To make the buffer mechanism of the output panel more robust.
Change description
Uses a setTimeout rather that setInterval in our buffer, this ensures that flush calls do not interfere with each other. On disposing our buffer we set a boolean flag to prevent additional setTimeout callbacks from being created, if there is a setTimeout callback in progress when the buffer is "disposed" it will complete before running flush a final time to be exhaustive.
gRPC errors are also added to the buffer, to ensure proper ordering in the output panel.
Reviewer checklist