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.
--
Mailing list: https://launchpad.net/~nagios-charmers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~nagios-charmers
More help : https://help.launchpad.net/ListHelp
