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

Remove Value from Http #2806

Merged
merged 1 commit into from
Mar 28, 2024
Merged

Remove Value from Http #2806

merged 1 commit into from
Mar 28, 2024

Conversation

GnomedDev
Copy link
Member

@GnomedDev GnomedDev commented Mar 18, 2024

This avoids allocating serde_json::Value

@github-actions github-actions bot added model Related to the `model` module. http Related to the `http` module. labels Mar 18, 2024
@GnomedDev GnomedDev force-pushed the next branch 3 times, most recently from aa90cc6 to 276c192 Compare March 25, 2024 22:27
@github-actions github-actions bot added the builder Related to the `builder` module. label Mar 25, 2024
@GnomedDev GnomedDev marked this pull request as ready for review March 25, 2024 23:07
@arqunis arqunis added enhancement An improvement to Serenity. breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users labels Mar 28, 2024
@arqunis arqunis merged commit 8692c34 into serenity-rs:next Mar 28, 2024
21 checks passed
@GnomedDev GnomedDev deleted the remove-value branch March 28, 2024 13:05
GnomedDev added a commit that referenced this pull request Mar 29, 2024
This avoids allocating instances of `serde_json::Value`s.
Comment on lines -180 to +182
http.edit_role_position(guild_id, role.id, position, self.audit_log_reason).await?;
guild_id
.edit_role_position_with_reason(http, role.id, position, self.audit_log_reason)
.await?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This introduced a dependency on the model feature inside code that is normally just gated behind http plus builder. Is it possible to fix this?

Copy link
Member Author

@GnomedDev GnomedDev Mar 30, 2024

Choose a reason for hiding this comment

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

I'm going to make a PR soon that moves builder inside of model and gets rid of the builder feature, since builder shouldn't be used without model anyway.

Copy link
Collaborator

@mkrasnitski mkrasnitski Mar 30, 2024

Choose a reason for hiding this comment

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

Can you elaborate? I'm thinking of the builder-no-http usecase as a specific counter-example to that idea.

EDIT: It would be feasible to gate the execute methods behind model instead of just http, but the structs themselves I don't think should be gated behind model.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's move this to the lib-development chat as this is going quite off-topic for this PR.

GnomedDev added a commit that referenced this pull request Mar 31, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Mar 31, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Apr 1, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request May 14, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit that referenced this pull request May 14, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit that referenced this pull request May 23, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit that referenced this pull request May 28, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Jun 9, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit that referenced this pull request Jun 22, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Jun 22, 2024
This avoids allocating instances of `serde_json::Value`s.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jul 29, 2024
This avoids allocating instances of `serde_json::Value`s.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jul 30, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit that referenced this pull request Aug 16, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Oct 7, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Oct 20, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit that referenced this pull request Oct 20, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Nov 11, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit that referenced this pull request Nov 13, 2024
This avoids allocating instances of `serde_json::Value`s.
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Nov 15, 2024
This avoids allocating instances of `serde_json::Value`s.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Dec 8, 2024
This avoids allocating instances of `serde_json::Value`s.
arqunis pushed a commit to arqunis/serenity that referenced this pull request Jan 16, 2025
This avoids allocating instances of `serde_json::Value`s.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Feb 1, 2025
This avoids allocating instances of `serde_json::Value`s.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Feb 14, 2025
This avoids allocating instances of `serde_json::Value`s.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users builder Related to the `builder` module. enhancement An improvement to Serenity. http Related to the `http` module. model Related to the `model` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants