Comment 3 for bug 920797

Hi David, thanks again for taking the time to work on and submit this charm. Starting off I ran `juju charm proof` from charm-tools and your charm came back with a clean lint! Way to go!

# Review

After a deeper review, there are a few things that need to be addressed. Your readme is very light, since this is what people will read when deploying your charm we prefer them to be quite verbose in what they detail. We recommend you outline each configuration option and what it does (with examples how to set), outline any relations the charm has, demonstrate as many preferred workflows for the charm, how to scale the service, contact information, a general overview, etc.

Your upgrade-charm won;t actually work. All hooks CWD are set to the charm's root ($CHARM_DIR), so you'd need to invoke each with hooks/install and hooks/config-changed though you've got the right idea as far as handling upgrades to the charm.

In your start hook you open-port 7777, but since the port is configurable you'd need to `open-port $(config-get port)`

Your copyright currently assigns copyright to Canonical. I presume you meant to assign copyright to yourself instead. Consider updating this as to not just give away your intellectual property and hard work!

# Knitpicks

I also noticed that the charm has no database relations, though the service has support for a Redis backend. While this isn't a blocker, but having a relation for the redis interface would be a good way to make this service scalable. Another alternative would be to implement the "mount" interface so a user could scale the service and use NFS to connect the servers filestore. Neither of these are a blocker, but something you might want to consider to make your charm scale-out safe.

Some minor things, the tic/tac brackets aren't needed in metadata.yaml (<>) for the description and summary, consider moving the upstart file from a HEREDOC to an actual file in the charm (say, in a directory called "files", or whatever) then just have the install hook copy the file in to place. It'll make the install hook a little easier to read and you're not using any variables so it doesn't benefit from being a HEREDOC.

Outside of that, the charm looks great! We really appreciate the time and effort you've taken in to getting haste deployable on Ubuntu through Juju. If you have any questions about the review, or of Juju/charms in general feel free to reply to this thread, find us in #juju on freenode.net, mail us at <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com

Pending fixes for the critical items outlined above I've moved this bug to incomplete. Please move to either New or Fix Committed to have it re-added to the queue and either myself or another charmer will be by shortly after with another review!