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.
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.