Skip to content

Remove LegacyMembershipManager #4862

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

Merged
merged 7 commits into from
Jun 26, 2025
Merged

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Jun 3, 2025

The legacy membership manager was kept around during the transition phase. It is now in a spot where we dont use it anymore and where we are getting better results with the new implementation.

This removes all related code and deprecates fields related to it so that downstream projects can adjust. (in particular: useNewMembershipManager)

There was a custom test file that tested both (new and legacy implementation) this file used describe.each and now just became a simple describe. All helper code for this setup also got removed.

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

Sorry, something went wrong.

@richvdh richvdh changed the title Remove LegacyMemberhsipManager Remove LegacyMembershipManager Jun 4, 2025
@toger5 toger5 added the T-Task Tasks for the team like planning label Jun 10, 2025
toger5 added 3 commits June 10, 2025 14:54
Those tests were only run with the legacy membership manager and are redundant with the memberhsip manager test spec.
@toger5 toger5 force-pushed the toger5/remove-legacymemebershipmanager branch from 8de7202 to 5d9dfc9 Compare June 10, 2025 12:54
@toger5 toger5 marked this pull request as ready for review June 10, 2025 13:40
@toger5 toger5 requested a review from a team as a code owner June 10, 2025 13:40
@toger5 toger5 requested a review from robintown June 10, 2025 13:40
expect(sendDelayedStateMock).toHaveBeenCalledTimes(1);
}

it("sends events", async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are this and the tests below it being removed because they never were testing the new membership manager?

@@ -77,6 +76,7 @@ export interface MembershipConfig {
* Use the new Manager.
*
* Default: `false`.
* @deprecated does nothing anymore we always default to the new memberhip manager.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so this isn't even a breaking change as we're not exporting the membership managers to begin with. Nice 👍

@toger5 toger5 added this pull request to the merge queue Jun 26, 2025
Merged via the queue into develop with commit 4f9ca2c Jun 26, 2025
32 checks passed
@toger5 toger5 deleted the toger5/remove-legacymemebershipmanager branch June 26, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants