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

fix:Add the output data of command 'info keyspace 0' to the output of com… #2603

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

XiaoLiang2333
Copy link
Contributor

@XiaoLiang2333 XiaoLiang2333 commented Apr 16, 2024

Add 'info keyspace 0' output data to 'info all' #2587

@github-actions github-actions bot added Invalid PR Title ✏️ Feature New feature or request labels Apr 16, 2024
@XiaoLiang2333 XiaoLiang2333 changed the title Add the output data of command 'info keyspace 1' to the output of com… fix:Add the output data of command 'info keyspace 1' to the output of com… Apr 16, 2024
@baerwang baerwang self-requested a review April 17, 2024 08:45
@XiaoLiang2333 XiaoLiang2333 marked this pull request as draft April 17, 2024 11:22
@baerwang baerwang marked this pull request as ready for review April 17, 2024 14:12
@github-actions github-actions bot added the ☢️ Bug Something isn't working label Apr 17, 2024
@XiaoLiang2333 XiaoLiang2333 changed the title fix:Add the output data of command 'info keyspace 1' to the output of com… fix:Add the output data of command 'info keyspace 0' to the output of com… Apr 18, 2024
@baerwang baerwang removed their request for review April 19, 2024 09:22
tmp_stream << "# Use \"info keyspace 1\" do async statistics"
<< "\r\n";
}

**/
Copy link
Collaborator

Choose a reason for hiding this comment

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

删除

} else {
tmp_stream << "# Use \"info keyspace 1\" to do async statistics\r\n"; // Command=>"info"
}
/**if (argv_.size() == 3){ // command => `info keyspace 1` or `info all`
Copy link
Collaborator

Choose a reason for hiding this comment

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

不要的代码直接删除即可

Remove obsolete code
@@ -1194,15 +1196,19 @@ void InfoCmd::InfoKeyspace(std::string& info) {
std::stringstream tmp_stream;
tmp_stream << "# Keyspace"
<< "\r\n";
if (argv_.size()> 1 && strcasecmp(argv_[1].data(), kAllSection.data()) == 0) {
Copy link
Collaborator

@Mixficsol Mixficsol Apr 28, 2024

Choose a reason for hiding this comment

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

  if (argv_.size() > 1 && strcasecmp(argv_[1].data(), kAllSection.data()) == 0) {
    tmp_stream << "# Start async statistics\r\n";
  } else if (argv_.size() == 3 && strcasecmp(argv_[1].data(), kKeyspaceSection.data()) == 0) {
    tmp_stream << "# Start async statistics\r\n";    
  } else {
    tmp_stream << "# Use \"info keyspace 1\" to do async statistics\r\n";  
  }

这样写是不是更好一些

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,我过于执着分支了,你的代码健壮性和清晰度都更好

Copy link
Collaborator

Choose a reason for hiding this comment

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

过奖了

@chejinge chejinge merged commit ed0b4fe into OpenAtomFoundation:unstable Apr 29, 2024
12 checks passed
chejinge pushed a commit that referenced this pull request May 15, 2024
… com… (#2603)

* Add the output data of command 'info keyspace 1' to the output of command 'info all'
chenbt-hz pushed a commit to chenbt-hz/pika that referenced this pull request Jun 3, 2024
… com… (OpenAtomFoundation#2603)

* Add the output data of command 'info keyspace 1' to the output of command 'info all'
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
… com… (OpenAtomFoundation#2603)

* Add the output data of command 'info keyspace 1' to the output of command 'info all'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.5.4 4.0.0 ☢️ Bug Something isn't working ✏️ Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants