Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Replace concast.cleanup method with simpler concast.beforeNext API #9718

Merged
merged 4 commits into from
May 13, 2022
Merged

Replace concast.cleanup method with simpler concast.beforeNext API #9718

merged 4 commits into from
May 13, 2022

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented May 13, 2022 •
edited
Loading

@benjamn benjamn added this to the v3.6.x patch releases milestone May 13, 2022
@benjamn benjamn self-assigned this May 13, 2022
@benjamn benjamn force-pushed the issue-9690-eliminate-concast.cleanup branch from eea2030 to f3f55e4 Compare May 13, 2022 20:17
@benjamn benjamn requested a review from StephenBarlow as a code owner May 13, 2022 20:17
@benjamn benjamn changed the base branch from release-3.7 to main May 13, 2022 20:17
benjamn added 4 commits May 13, 2022 17:58
@benjamn
Implement concast.beforeNext as replacement for concast.cleanup.
c1d0a91 
Doing this right would potentially resolve issue #9690.
@benjamn
Bump bundlesize limit to 29.5kB (now 28.46kB).
f6848cc
@benjamn
More tests of concast.beforeNext.
76af72d
@benjamn
Remove quietly parameter from Concast#removeObserver method.
6fb1289 
I could swear I tried this before and it broke a bunch of tests, but
tests are passing without it.
@benjamn benjamn force-pushed the issue-9690-eliminate-concast.cleanup branch from 5446438 to 6fb1289 Compare May 13, 2022 22:00
@benjamn benjamn changed the base branch from main to release-3.7 May 13, 2022 22:00
@benjamn
Copy link
Member Author

benjamn commented May 13, 2022 •
edited
Loading

Merging into release-3.7 without review because I want to get feedback on whether this helps with #9690, and bcause the new unit tests I added are passing.

If this PR solves #9690, then I would say the quietly parameter was to blame for most of the original problem.

@benjamn benjamn merged commit 95797ae into release-3.7 May 13, 2022
@benjamn benjamn deleted the issue-9690-eliminate-concast.cleanup branch May 13, 2022 22:04
benjamn added a commit that referenced this pull request May 13, 2022
@benjamn
Mention PR #9718 in CHANGELOG.md.
88b18e5
This was referenced May 13, 2022
Closed
Closed
@benjamn benjamn requested a review from brainkim May 16, 2022 21:11
This was referenced Jun 10, 2022
Merged
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@StephenBarlow StephenBarlow Awaiting requested review from StephenBarlow

@brainkim brainkim Awaiting requested review from brainkim

Assignees

@benjamn benjamn

Labels
🧩 implementation-detail 🛫 in-progress 👩‍🔬 needs-more-tests
Projects
None yet
Milestone
v3.x.x patch releases
Development

Successfully merging this pull request may close these issues.
1 participant
@benjamn