Comment 1 for bug 1356856

Revision history for this message
Antonio Rosales (arosales) wrote :

Marco,

Thanks for your work on the Nginx charm. I know this is a highly requested charm, and will be a good addition to the Juju Charm store as a recommended charm. To others following along this charm can be deploy by:

juju deploy cs:~hp-discover/trusty/nginx

At first glance this charm is in the "Incomplete" status, but still shows up in the review queue so that needs to be addressed by the review-queue. I filled https://github.com/marcoceppi/review-queue/issues/9 to address this and the lack of the auto-charm testing bot on this bug.

My review is below
----

# Charm Proof
Charm Proof fails with the following warnings:
$ charm proof nginx/
W: README.md includes line 1 of boilerplate README.ex
W: README.md includes line 2 of boilerplate README.ex
W: README.md includes line 3 of boilerplate README.ex
W: README.md includes line 4 of boilerplate README.ex
W: README.md includes line 5 of boilerplate README.ex
W: README.md includes line 6 of boilerplate README.ex
W: README.md includes line 7 of boilerplate README.ex
W: README.md includes line 8 of boilerplate README.ex
W: README.md includes line 9 of boilerplate README.ex
W: README.md includes line 10 of boilerplate README.ex
W: README.md includes line 11 of boilerplate README.ex
W: README.md includes line 12 of boilerplate README.ex
W: README.md includes line 13 of boilerplate README.ex
W: README.md includes line 14 of boilerplate README.ex

# Readme and metadata
The Readme looks largely to be the boilerplate template. The copyright and metadata.yaml file did look to be updated though.

# Icon
Icon looks good.

# Tests
This branch does not include any tests. In order to be accepted as a Trusty recommended charm this charm will need to have tests.

# Deploy
The charm successfully goest to a "started" state when deploying on local provider. As the Readme is incomplete I did not have opportunity to further test per the author's recommendation.

# Summary
This charm looks very close to being a recommended Trusty charm. In order to pass review the following issues need to be address:
  -Complete Readme
  -charm tests

Thanks for your work on this charm. Please move the bug status to "Fix Committed" when you are ready for another review.

-thanks,
Antonio