-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Use CLI11 to provide proper argument/option parsing to netdatacli #19099
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Largely LGTM other than the couple of review comments.
) | ||
else() | ||
FetchContent_Declare(CLI11 | ||
GIT_REPOSITORY https://github.com/microsoft/mimalloc.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mimalloc should be it’s own thing unless it’s absolutely required by CLI11.
app.set_help_flag("--help", "Show this help message and exit"); | ||
app.add_flag("--reload-health", reload_health, "Reload health configuration"); | ||
app.add_flag("--reload-labels", reload_labels, "Reload all labels"); | ||
app.add_flag("--reopen-logs", reopen_logs, "Close and reopen log files"); | ||
app.add_flag("--shutdown-agent", shutdown_agent, "Cleanup and exit the netdata agent"); | ||
app.add_flag("--fatal-agent", fatal_agent, "Log the state and halt the netdata agent"); | ||
app.add_flag("--reload-claiming-state", reload_claiming_state, "Reload agent claiming state from disk"); | ||
app.add_flag("--ping", ping, "Return with 'pong' if agent is alive"); | ||
app.add_option( | ||
"--aclk-state", aclk_state, "Returns current state of ACLK and Cloud connection. Use 'json' for JSON format"); | ||
app.add_flag("--dump-config", dumpconfig, "Returns the current netdata.conf on stdout"); | ||
app.add_option( | ||
"--remove-stale-node", | ||
remove_stale_node, | ||
"Unregisters and removes a node from the cloud. Specify node_id, machine_guid, hostname, or ALL_NODES"); | ||
app.add_flag("--version", version, "Returns the netdata version"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing all of these to use a --
prefix is a breaking change. Ideally we need to keep the old syntax without the --
to avoid breaking things (though I would like to see --help
supported alongside help
, and probably ideally also a --version
option since we should be able to provide that).
include(FetchContent) | ||
include(NetdataFetchContentExtra) | ||
|
||
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.28) | ||
FetchContent_Declare( | ||
CLI11 | ||
GIT_REPOSITORY https://github.com/CLIUtils/CLI11.git | ||
GIT_TAG 6c7b07a878ad834957b98d0f9ce1dbe0cb204fc9 # v2.4.2 | ||
EXCLUDE_FROM_ALL | ||
) | ||
else() | ||
FetchContent_Declare(CLI11 | ||
GIT_REPOSITORY https://github.com/microsoft/mimalloc.git | ||
GIT_REPOSITORY https://github.com/CLIUtils/CLI11.git | ||
GIT_TAG 6c7b07a878ad834957b98d0f9ce1dbe0cb204fc9 # v2.4.2 | ||
) | ||
endif() | ||
|
||
FetchContent_MakeAvailable_NoInstall(CLI11) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought our goal was to minimize the amount of stuff like this in the main CMakeLists.txt file?
Summary
SSIA.
Test Plan
Additional Information
Needs more work to update references of
netdatacli
throughout the codebase.