Happy 2015! I hope everyone new year is off to a good start. This week I spent some time on the following reviews:
OpenID/SSO Subordinate Charm Nice charm from the PES Tools & Certification Canonical team. My major comments were around adding tests and sprucing up the readme a bit. Pending adding charm tests this charm looks to be ready for a final review by a charmer and acceptance into the recommeded charm store. https://bugs.launchpad.net/charms/+bug/1396237 Pinged Marco on ensuring the following charms dropped out of the review queue: Trusty Lamp: https://bugs.launchpad.net/charms/+bug/1387467 Trusty pythin-django: https://bugs.launchpad.net/charms/+bug/1387465 The following charms had MPs for adding tests. https://code.launchpad.net/~marcoceppi/charms/precise/seafile/tests/+merge/240964 https://code.launchpad.net/~marcoceppi/charms/precise/opentsdb/tests/+merge/240984 https://code.launchpad.net/~marcoceppi/charms/precise/thinkup/tests/+merge/240894 https://code.launchpad.net/~mbruzek/charms/precise/newrelic-php/tests/+merge/240983 https://code.launchpad.net/~tvansteenburgh/charms/trusty/rabbitmq-server/add-amulet-test/+merge/245766 https://code.launchpad.net/~nicopace/charms/trusty/rabbitmq-server/10-outofthebox-testing/+merge/245871 Even though the charm is failing tests I suggest to accept these charm test MP for the reasons listed in the MPs. My overall reasoning is as follows: 1. A test is still valid even if the results aren't positive. 2. Having charm tests provides the first step to helping the charm author and community fix the charm. Specifically by providing a reproducible failure. 3. The tests are seed (basic) tests that an interested person can build off of. 4. As fixes are made to this charm automating charm testing will be able to run additional tests in addition to charm proof. 5. Having basic tests, as demonstrated here, helps ensure the charm is in a working (deployable) state given this charm is a recommended charm. A test failure can prompt a bug the charm author can take action on. The charm author may not have been aware of the bug had the charm not had even basic deployment tests. I also added specific comments for each MP. In discussion with ~charmers in #juju the folks who responded were a +1 as long as the test itself was valid and not missing any config values since the tests were auto-generated. thanks, -- Antonio Rosales Juju Ecosystem Canonical -- Juju mailing list Juju@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju
