-
Notifications
You must be signed in to change notification settings - Fork 27.9k
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
eng: reapply test leak fixes #190890
eng: reapply test leak fixes #190890
Conversation
Re-applies #190623 which I merged before everyone had a chance to review. Alex and Ben, you were the two people whose code this touches and didn't review it in the original PR.
Looks good to me! Thanks for improving all of 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.
I do not feel confident about the changes to listenStream
, so I suggest to move those changes out of this PR. Maybe rather than returning a IDisposable
, the method listenStream
should optionally allow you to pass in a CancellationToken
.
Left some minor feedback for the rest.
Re-applies #190623 which I merged before everyone had a chance to review.
Alex and Ben, you were the two people whose code this touches and didn't
review it in the original PR.