Skip to content

Commit

Permalink
Make slack_webhook as non optional parameter
Browse files Browse the repository at this point in the history
Initial the CLI argument slack_webhook was optional, even though other
options related to notificatin were required to be passed (Thanks to
Kirill for raising this point to me):

- app name
- app version
- notification context

I initially made the slack_webhook option to convey the fact that the
notification system is pluggable and you could instead optionally
integrate other providers like Discord or Microsoft teams. Making it
as non optional would indicate that this tool is tied to slack.

I discovered that this problem is solved by clap's ArgGroup, so I have
converted the solution to that. This also makes sure that we pass
atleast one parameter of the NotifyHook struct.
  • Loading branch information
psibi committed Jul 25, 2024
1 parent 744a410 commit 232be30
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ pub(crate) struct Cli {
/// Seconds to wait for output before killing the task
#[arg(long)]
pub(crate) task_output_timeout: Option<u64>,
/// Slack Webhook for notification
#[arg(long, value_parser(Url::from_str), env = "HEALTH_CHECK_SLACK_WEBHOOK")]
pub(crate) slack_webhook: Option<Url>,
/// Notification hook
#[command(flatten)]
pub(crate) notification_hook: NotifyHook,
/// Application description
#[arg(long)]
pub(crate) app_description: String,
Expand All @@ -54,6 +54,13 @@ pub(crate) struct Cli {
pub(crate) output_lines: usize,
}

#[derive(clap::Args)]
#[group(required = true, multiple = false)]
pub(crate) struct NotifyHook {
#[arg(long, value_parser(Url::from_str), env = "HEALTH_CHECK_SLACK_WEBHOOK")]
pub(crate) slack_webhook: Option<Url>,
}

#[derive(Debug)]
enum MainMessage {
Error(anyhow::Error),
Expand Down Expand Up @@ -184,7 +191,7 @@ impl Cli {
match res {
Ok(()) => Ok(()),
Err(e) => {
if let Some(slack_webhook) = self.slack_webhook {
if let Some(slack_webhook) = self.notification_hook.slack_webhook {
let slack_app = SlackApp::new(
slack_webhook,
self.notification_context,
Expand Down

0 comments on commit 232be30

Please sign in to comment.