Comment 8 for bug 898714

Hi Ben!

Thanks for updating the charm. Here is some feedback on it:

== Blockers ==
- We recently added a requirement for charms to have a maintainer field in the metadata.yaml
  It would look something like:
  maintainer: Benjamin Kerensa <email address hidden>
  As a rule of thumb, it's a good idea to run charm proof ( charm proof <charm> ) before submitting.
  It tries to keep up with changes to the rules as well as new things we find need to be tested.

- When deploying the charm, the install hook fails with the following:
  node.js:201
          throw e; // process.nextTick error, or 'error' event on first tick
  Error: Cannot find module 'async'
  <......>

== Non-blockers ==
- hooks/install line 13
  Please consider moving the port number from a hard-coded value in install to a value in config.yaml.
  This will allow users to change the port number if they need/want to.
  An example of a config.yaml in use can be found here: lp:~charmers/charms/precise/mongodb/trunk
  Check it out and consider adding a config.yaml to your charm.

- hooks/install line 15-17
  Please consider using the existing MongoDB charm ( lp:~charmers/charms/precise/mongodb/trunk ).
  This could be a really good opportunity to showcase how different charms within the Juju ecosystem
  work together.

- hooks/locker-relation-joined line 1
  Please consider not hardcoding the port number here ... once you put the port number in config.yaml,
  you can use config-get <variable> to get the current value.

- There seems to be an orphan bin directory ... I don't see any reason for it to be there.

All in all, this looks good. After you fix the Blockers, I would be happy to review it again.

Thanks,

Juan