Comment 2 for bug 1375685

Revision history for this message
Charles Butler (lazypower) wrote :

Greetings Chris,

Thank you for the submission of the light weight logstash forwarder charm! I love the idea of having a log shipping agent that doesn't ship with a java dependency and will be following the development of this charm pretty closely. I've taken a moment to review the charm as is and have the following notes:

# Proof
The charm passes our automated testing that runs proof and any linting targets, which is great

# Missing Tests
For trusty based charms, we've started enforcing a policy update that trusty charms must include tests for our automated CI infrastructure. To learn more about the charm testing story give this post a quick read: http://blog.juju.solutions/cloud/juju/2014/10/02/charm-testing.html

This should help explain the why, and how to get moving quickly with charm testing. Along that veign there will also be a charm school today held at 3pm EDT over getting started with charm testing.

# Readme
The readme is very straight forward, it would be nice to see the addition of upstream contact information for the project should users become interested in following the project, or contributing to the further development of the charm/service based on their interaction with the Juju Charm. Additionally if you were to provide the Readme in markdown format, it renders much nicer in the charm store.

There are quite a few config options exposed in config.yaml that are not represented in the README, and it would be helpful to display proposed deployment workloads. Think brand new user vs a user that fully grokks Juju.

# Deploy Test
When deploying the charm 'as is' - I received an error during install: AttributeError: Config instance has no attribute 'apt_repository' - which appears to be related to not having anything specified in the config. This falls under sane defaults for deployment and warrants updating.

As an initial look this is a good first cut of the charm, and barring some modifications will be ready for the store in short order. We appreciate the submission and look forward to the next cut of the charm. I'm going to move the status of this bug to 'incomplete', and when you are ready for another review simply change the status to 'fix committed' and someone will be along shortly to review your work.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.