Code review comment for ~szeestraten/charm-nagios:bug/1864968

Revision history for this message
Sandor Zeestraten (szeestraten) wrote :

Thanks for the review Xav!

I prepended `_admin` and added comment in description to clarify it is for the default admin contact.

I considered adding similar config options for the pagerduty, but unfortunately it seems that the config option name `pagerduty_notification_levels` was chosen for the `service_notification_options` which breaks the naming convention and changing it for existing deployments is probably not the scope for this bug/patch.

Also commented that the notification_commands is for changing or adding custom notification plugins which is the intended use case for those commands.

« Back to merge proposal