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.

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

Replace usage of CacheAndHttp in dispatch with Context #2197

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

mkrasnitski
Copy link
Collaborator

This is part of an effort to split #2118 up to address some of the concerns raised there. This PR changes dispatch such that it passes a context around instead of constructing one over and over, simplifying method signatures and reducing the number of clones (the clones themselves were cheap anyways because it was just Arc cloning, which means just incrementing a refcount).

The whole DispatchEvent::update_cache thing is still really ugly, as its only used in the absence of an non-raw event handler. Ideally I'd like to get rid of it somehow.

@kangalio
Copy link
Collaborator

kangalio commented Oct 3, 2022

FYI this is a bit annoying to review due to many changes done in one commit. I've pulled them apart here: 1. e7f63db, 2. 3322570

@mkrasnitski
Copy link
Collaborator Author

Ah yeah, sorry. The dispatch function itself didn't change much, I just removed the calls to context() and changed the update() calls.

@kangalio kangalio mentioned this pull request Oct 3, 2022
@arqunis arqunis merged commit 98e3aad into serenity-rs:next Oct 3, 2022
@arqunis arqunis added the enhancement An improvement to Serenity. label Oct 3, 2022
@mkrasnitski mkrasnitski deleted the cleanup_dispatch branch October 3, 2022 22:03
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Nov 7, 2022
arqunis pushed a commit to arqunis/serenity that referenced this pull request Nov 13, 2022
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Feb 28, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 18, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 30, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 17, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 24, 2023
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 24, 2023
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Related to the `client` module. enhancement An improvement to Serenity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants