Skip to content

refactor: conversation_mut takes a closure [WPB-23149]#2048

Merged
coriolinus merged 4 commits intomainfrom
prgn/refactor/23149-conversation-mut-callback
Apr 21, 2026
Merged

refactor: conversation_mut takes a closure [WPB-23149]#2048
coriolinus merged 4 commits intomainfrom
prgn/refactor/23149-conversation-mut-callback

Conversation

@coriolinus
Copy link
Copy Markdown
Contributor

What's new in this PR

This affects our internal API only, no publicly-visible changes.


PR Submission Checklist for internal contributors
  • The PR Title
    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

@coriolinus coriolinus requested a review from a team April 21, 2026 07:36
Copy link
Copy Markdown
Contributor

@fewerner fewerner left a comment

Choose a reason for hiding this comment

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

I like it a lot! I let Simon finish his review and approve it.

Comment thread crypto/src/mls/conversation/conversation_guard/mod.rs Outdated
Comment thread crypto/src/mls/conversation/conversation_guard/merge.rs Outdated
@fewerner fewerner changed the title refactor: conversation_mut takesa a closure [WPB-23149] refactor: conversation_mut takes a closure [WPB-23149] Apr 21, 2026
Comment thread crypto/src/mls/conversation/conversation_guard/decrypt/mod.rs
Copy link
Copy Markdown
Member

@SimonThormeyer SimonThormeyer left a comment

Choose a reason for hiding this comment

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

Looks good. Nothing to add from my side (except maybe that 37ee969 could be folded into the commits that introduced the need for it – up to you).

This ensures we can't ever forget to call `persist_group_when_changed`.

A more natural pattern in Rust would have been to return a lightweight
guard type which calls `persist_group_when_changed` on `Drop`, but that
doesn't work for two reasons:

1. We don't actually want to call it when an operation on the conversation
   has failed.
2. `Drop` isn't async.
@coriolinus coriolinus force-pushed the prgn/refactor/23149-conversation-mut-callback branch from 5a15dca to 6ef84f9 Compare April 21, 2026 13:53
@coriolinus coriolinus merged commit 6ef84f9 into main Apr 21, 2026
33 checks passed
@coriolinus coriolinus deleted the prgn/refactor/23149-conversation-mut-callback branch April 21, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants