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

chore: get rid of MutableSlice #3952

Merged
merged 2 commits into from
Oct 23, 2024
Merged

chore: get rid of MutableSlice #3952

merged 2 commits into from
Oct 23, 2024

Conversation

romange
Copy link
Collaborator

@romange romange commented Oct 19, 2024

No description provided.

@romange romange force-pushed the Pr4 branch 4 times, most recently from 9f2b78d to 150b5a2 Compare October 20, 2024 16:55
@romange romange marked this pull request as ready for review October 20, 2024 16:56
@romange romange requested a review from dranikpg October 20, 2024 16:56
Signed-off-by: Roman Gershman <[email protected]>
dranikpg
dranikpg previously approved these changes Oct 20, 2024
Copy link
Contributor

@dranikpg dranikpg left a comment

Choose a reason for hiding this comment

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

  1. Please be attentive to places like this
    for (string_view channel : ArgS(args)) {

Where we cast MutableSlice to ArgSlice to iterate over it, we don't need it now

  1. I'm not sure all copies were removed or there aren't any more places that can be simplified now

@@ -39,14 +39,12 @@ struct Entry : public EntryBase {
// Payload represents a non-owning view into a command executed on the shard.
struct Payload {
std::string_view cmd;
std::variant<CmdArgList, // Parts of a full command.
ShardArgs, // Shard parts.
std::variant<ShardArgs, // Shard parts.
Copy link
Contributor

Choose a reason for hiding this comment

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

you lost a comment here

@romange romange merged commit 4aa0ca1 into main Oct 23, 2024
12 checks passed
@romange romange deleted the Pr4 branch October 23, 2024 18:50
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.

2 participants