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 (#1587): show pika version #1588

Merged
merged 1 commit into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ output/
# DB
db/
dump/
src/dbsync/

# third party
gdb.txt
Copy link

Choose a reason for hiding this comment

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

Based on the code patch you provided, it seems that all you did was add a new directory called src/dbsync/ to your version control system.

As such, there is no code that needs reviewing for bugs or improvement suggestions. However, if you have any implementation concerns related to the contents of this new directory, please provide more information and I would be happy to help.

Expand Down
4 changes: 2 additions & 2 deletions src/pika.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "include/pika_version.h"
#include "pstd/include/env.h"


std::unique_ptr<PikaConf> g_pika_conf;
// todo : change to unique_ptr will coredump
PikaServer* g_pika_server;
Expand Down Expand Up @@ -67,7 +66,7 @@ static void daemonize() {
if (fork()) {
exit(0); /* parent exits */
}
setsid(); /* create a new session */
setsid(); /* create a new session */
}

static void close_std() {
Expand Down Expand Up @@ -120,6 +119,7 @@ static void usage() {
"usage: pika [-hv] [-c conf/file]\n"
"\t-h -- show this help\n"
"\t-c conf/file -- config file \n"
"\t-v -- show version\n"
AlexStocks marked this conversation as resolved.
Show resolved Hide resolved
" example: ./output/bin/pika -c ./conf/pika.conf\n",
version);
}
Copy link

Choose a reason for hiding this comment

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

Overall, the code patch seems straightforward and free of major issues. Below are a few minor suggestions for improvement:

  1. Remove unnecessary whitespace in Line 7 for better readability.
  2. Consider adding comments to describe the function and purpose of the daemonize() and close_std() functions.
  3. It may be helpful to provide more details about the version number in the usage string. For example, specifying whether it is the version of the program or its dependencies.

As for any bug risks, without further context on the larger program and its dependencies, it is difficult to determine if there are any critical implications to this code patch.

Expand Down