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

Refactor Membership store interface #6309

Merged
merged 3 commits into from
Jun 7, 2023
Merged

Refactor Membership store interface #6309

merged 3 commits into from
Jun 7, 2023

Conversation

nicholaspcr
Copy link
Contributor

Summary

References #5587

Changes

  • Separate DeleteMember into its own method

Testing

Unit tests

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@nicholaspcr nicholaspcr added the technical debt Not necessarily broken, but could be done better/cleaner label Jun 5, 2023
@nicholaspcr nicholaspcr self-assigned this Jun 5, 2023
@nicholaspcr nicholaspcr requested a review from adriansmares as a code owner June 5, 2023 15:14
@github-actions github-actions bot added c/identity server This is related to the Identity Server compat/db This could affect Database compatibility labels Jun 5, 2023
adriansmares
adriansmares approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@adriansmares adriansmares left a comment

Choose a reason for hiding this comment

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

An earlier iteration that I saw also added a new RPC. While I am ok with dropping that, I am curious why.

@adriansmares adriansmares added this to the v3.26.1 milestone Jun 6, 2023
@nicholaspcr
Copy link
Contributor Author

An earlier iteration that I saw also added a new RPC. While I am ok with dropping that, I am curious why.

Should've written on the section for reviewers, I was just planning on doing it on a separate issue for the rpc implementations, since the issue itself is just about the interface refactors and having everything in one branch was too big.

@nicholaspcr nicholaspcr merged commit d461e46 into v3.26 Jun 7, 2023
@nicholaspcr nicholaspcr deleted the refactor/5587-membership-store branch June 7, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriansmares adriansmares adriansmares approved these changes

Assignees

@nicholaspcr nicholaspcr

Labels
c/identity server This is related to the Identity Server compat/db This could affect Database compatibility technical debt Not necessarily broken, but could be done better/cleaner
Projects
None yet
Milestone
v3.26.1
Development

Successfully merging this pull request may close these issues.
2 participants
@nicholaspcr @adriansmares