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

Truncate cache if max_messages is set to a lower value at runtime #2957

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Truncate cache if max_messages is set to a lower value at runtime #2957

merged 1 commit into from
Aug 30, 2024

Conversation

ivinjabraham
Copy link
Contributor

Just a small commit that removes excess messages from cache if Settings::max_messages is changed at runtime through set_max_messages. Lot of iteration, but I couldn't think of a faster way to do it.

Address #2884.

@github-actions github-actions bot added the cache Related to the `cache`-feature. label Aug 26, 2024
@GnomedDev
Copy link
Member

Can this be retargetted and reimplemented on next? Although this is a non-breaking change, the layout of the MessageCache was simplified on next and isn't so fragile.

@github-actions github-actions bot added framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate model Related to the `model` module. client Related to the `client` module. builder Related to the `builder` module. http Related to the `http` module. utils Related to the `utils` module. gateway Related to the `gateway` module. command_attr Related to the `command_attr` crate. examples Related to Serenity's examples. ci Related to the continuous integration. labels Aug 29, 2024
@ivinjabraham ivinjabraham changed the base branch from current to next August 29, 2024 15:50
@github-actions github-actions bot removed framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate model Related to the `model` module. client Related to the `client` module. builder Related to the `builder` module. http Related to the `http` module. utils Related to the `utils` module. gateway Related to the `gateway` module. examples Related to Serenity's examples. ci Related to the continuous integration. labels Aug 29, 2024
@ivinjabraham
Copy link
Contributor Author

Re-implemented on next as requested. There were a few unintended commits from current pushed, which caused the bot to add the new labels.

GnomedDev
GnomedDev approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@GnomedDev GnomedDev left a comment

Choose a reason for hiding this comment

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

LGTM, but if you can just change to using VecDeque::drain instead of that manual pop_front loop that would be lovely.

@arqunis arqunis merged commit 1db2076 into serenity-rs:next Aug 30, 2024
21 checks passed
@GnomedDev GnomedDev removed the command_attr Related to the `command_attr` crate. label Aug 30, 2024
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Oct 7, 2024
@ivinjabraham @GnomedDev
Truncate cache if max_messages is set to a lower value at runtime (s…
4b4ffb1 
…erenity-rs#2957)

Fixes serenity-rs#2884
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Oct 20, 2024
@ivinjabraham @GnomedDev
Truncate cache if max_messages is set to a lower value at runtime (s…
0da7618 
…erenity-rs#2957)

Fixes serenity-rs#2884
GnomedDev pushed a commit that referenced this pull request Oct 20, 2024
@ivinjabraham @GnomedDev
Truncate cache if max_messages is set to a lower value at runtime (#…
9b3e747 
…2957)

Fixes #2884
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Nov 11, 2024
@ivinjabraham @GnomedDev
Truncate cache if max_messages is set to a lower value at runtime (s…
efe99e9 
…erenity-rs#2957)

Fixes serenity-rs#2884
GnomedDev pushed a commit that referenced this pull request Nov 13, 2024
@ivinjabraham @GnomedDev
Truncate cache if max_messages is set to a lower value at runtime (#…
18b580c 
…2957)

Fixes #2884
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Nov 15, 2024
@ivinjabraham @GnomedDev
Truncate cache if max_messages is set to a lower value at runtime (s…
9023e72 
…erenity-rs#2957)

Fixes serenity-rs#2884
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GnomedDev GnomedDev GnomedDev approved these changes

@arqunis arqunis arqunis approved these changes

Assignees
No one assigned
Labels
cache Related to the `cache`-feature.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ivinjabraham @GnomedDev @arqunis