Comment 2 for bug 1236112

Hi Paul, thanks again for taking the time to submit this charm to the charm store. I just ran `juju charm proof` from charm-tools against your charm with the following results:

W: No icon.svg file.
I: relation website has no hooks
W: config.yaml: option rabbit_password does not have the keys: default
W: config.yaml: option rabbit_host does not have the keys: default

We prefer our charms to have an icon, it really makes the Juju GUI and charm store experience a lot more compelling. Information on how to do this can be found on our docs: https://juju.ubuntu.com/docs/authors-charm-icon.html

You don't have any hooks for your website relation. Here's an example of all you'll need to properly implement the "http" interface for your charm: http://manage.jujucharms.com/~marcoceppi/precise/discourse/hooks/website-relation-changed

All configuration options should define a default key, even if that key is left blank/null (either as `default: ` or `default: ""`, preferably the latter to represent an empty string)

The readme is pretty light right now, typically we like when a README is very verbose, covering what each config option does, their defaults, what happens when you change settings, describe any relations and how they operate, describe caveats about the charm (does it scale? common use patterns, example workflows), contact information for the maintainer, etc.

You have a copyright file that assigns copyright of all files to Canonical. I'm guessing you copied this from another charm (not problem) but make sure you apply proper ownership (unless you want Canonical to own all your hard work ;)

dashboard-pass configuration option is currently immutable. Where if a user doesn't set it prior to deployment they can't change it. You'll need to implement logic in hooks/config-changed to properly handle configuration changes from the user via `juju set`

You set a default password for "dashboard_password" this is a bad practice and against store policy. Instead leave it blank and consider not starting the service until it's been set by the user.

# Knitpicks

The description should not be a description of the service, but more a description of what the charm does in the metadata.yaml

You have a VagrantFile in there (it looks like it does something pretty cool) but it's not documented in the README anywhere. This is something else to consider.

You don't need to use "sudo" anywhere in the hooks, you're running as root.

You install a redis-server, but you could actually model that as a redis relation if it makes sense (having multiple sensu-apps sharing one redis server during a scale out scenario for example)

You're git cloning stuff directly to /usr/local/share/ Not that this is nessisarily a bad thing, but you might want consider instead placing in /opt/ unless it *has* to live there

You open-ports during the start hook, but if there's no database connection will sensu work? If not you should open the ports in the ampq relation hook after a connection has been made.

I realize this is a lot of feedback, but I wanted to make sure I provided as much information as possible to help you during your next revision. All in all this charm is really shaping up and I look forward to seeing it in the charm store soon!

Let us know if you have any questions. You can reply to this thread, post to the mailing list <email address hidden>, find us in #juju on freenode.net, or ask a question on http://askubuntu.com

I've moved this to incomplete, when ready for another review please move to either "fix committed" or "new". Thanks again for your hard work so far!