Comment 2 for bug 956259

Hi Patrick

Thanks for submitting this charm for review.

Some feedback:

1) charm configuration:

There is alot of znc specific syntax/configuration in the config.yaml for this charm. I think it would be better to try to abstract the users of the charm away from the implementation details.

With this in mind its probably better to go with a single port configuration rather than one for each - I noticed that you strip out everything side from the number anyway in the install hook.

Makes the open-port command easier as well.

With regards to the use of XML in the config.yaml - I don't think this is a good idea.

Again I think this should be abstracted into a minimal set of data which the install hook uses to generate the configuration for znc; maybe username, nick, password, network and a list of channels. This is probably enough to get people bootstrapped.

They can then use the web interface to add additional configuration later.

2) start/stop hooks

As you install upstart configuration for znc these are not really required IMHO.

3) install

I had problems with:

su -c "/usr/bin/znc -d /var/lib/znc --makepem" znc

Running the local provider - this command was prompting for input - might be better to try to pass some sensible defaults to this as a fully qualified domain name is not always configured (I think this is the issue).

Generally it would be good to make this hook a bit more resilient to being executed multiple times.

Please address this feedback and mark this bug as 'Fix Committed' when its ready for review again.