nagios-charmers team mailing list archive

[Andrea Ieri <andrea.ieri@xxxxxxxxxxxxx>]

Review: Approve

Looks good. Just a very minor nit inline.
Also, for the future: whitespace fixes are welcome, but I think it would be cleaner to keep them in a separate 'noop' commit.

Diff comments:

> diff --git a/hooks/upgrade-charm b/hooks/upgrade-charm
> index 19e736c..d70a368 100755
> --- a/hooks/upgrade-charm
> +++ b/hooks/upgrade-charm
> @@ -361,8 +363,9 @@ update_localhost()
>  update_cgi_config()
>  update_password('nagiosro', ro_password)
>  if password:
> -    update_password('nagiosadmin', password)
> -
> +    update_password(nagiosadmin, password)
> +if nagiosadmin != 'nagiosadmin':

The intent here is not immediately clear. I suggest adding a small comment like "ensure we don't have a nagiosadmin user"

> +    update_password('nagiosadmin', False)
>  
>  subprocess.call(['hooks/mymonitors-relation-joined'])
>  subprocess.call(['hooks/monitors-relation-changed'])


-- 
https://code.launchpad.net/~aggkolaitis/nagios-charm/+git/nagios-charm/+merge/374089
Your team Nagios Charm developers is requested to review the proposed merge of ~aggkolaitis/nagios-charm:change_nagios_gui_username into nagios-charm:master.