Comment 3 for bug 894825

Revision history for this message
Mark Mims (mark-mims) wrote :

Review:

Looks great overall... just a couple of minor fixes before it goes into the charm store.

Things to fix:

- we need to cryptographically verify the payload of your wget. There're many ways to do this.. the easiest is if the upstream provider publishes hashes of their downloads... we just check the download against those published hashes. If upstream doesn't hash their downloads, you can just hash the payload yourself and compare that hash explicitly from the charm. Drawback is that when the payload changes versions, the hash will change and we'll have to update the charm and charm version accordingly.

- please push to lp:~brunopereira81/charm/oneiric/teamspeak3/trunk so the charm'll be picked up by the charm store browser.

- please go ahead and remove unimplemented relations from the metadata.yaml file... you can safely delete everything after the description.

Things to discuss and/or recommendations:

- Does it make sense to enhance the functionality of the /etc/init.d/teamspeak3 startup script to include functions like 'restart' or 'generate_database' instead of working those explicitly into the config-changed hook? Dunno... what you have looks like it works fine, just thinking about organization of responsibilities.

Testing spin-up now...

Thanks!
Mark