-
Notifications
You must be signed in to change notification settings - Fork 761
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
classlib: fix EventPatternProxy premature cleanup of faded-in stream #4996
classlib: fix EventPatternProxy premature cleanup of faded-in stream #4996
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.
thanks! test needs some work. our unit testing guide is here, by the way
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.
looks good, can you please rebase this branch to resolve the conflicts?
…(rebased) Rebased onto fba43fa
38ddcd0
to
e57b6d0
Compare
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.
thanks!
I'll leave a note here that person who first reported this bug says (on the forum link in the initial report) that this fix didn't work for them. I have no idea why. It worked for me in literally all the instances of SC I've patched with this, i.e. backport to 3.10.4 and 3.11.0. But they are on a Mac, and I don't have one I can install SC on, alas. Since we get no classlib unit test output from CI, it would be helpful if someone (else) with SC on a Mac can manually check if this fix working on that OS. |
Purpose and Motivation
#Fixes #4954
I have checked that fix this works with both the present cleanup system and the one proposed in #4806.
Types of changes
To-do list