Code review comment for lp://staging/~juju-qa/charms/precise/jenkins-slave/trunk

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

Greetings,

I've had a chance to review this great jenkins slave submission! I do have a few nit picks with commentary on how to improve, they are as follows:

The README, while now being present - is still extremely light on details. It would be nice to cover all the basics of a readme. If you have the charm-tools package installed, issue 'charm add readme' and fill out the pertinent portions of the readme that effect the jenkins-slave charm complete with but not limited to:

- Configuration
- Upstream Details on where to file bugs/ how to contact the maintainer
- adding the details from hooks/install.d into the main README
- Adding details on how to provide a newer DPKG than whats packaged in the charm

In the metadata.yaml, where James Page is still the listed maintainer. Is the Juju QA team taking over the Jenkins Charm suite? If so, it would be brilliant if you could set the QA team as the maintainer of the charm in the metadata.

While I like the concept of fat charms and using the deb packaged within the charm itself, it would be another great addition to add the option for installation from a PPA - such as outlined here: https://wiki.jenkins-ci.org/display/JENKINS/Installing+Jenkins+on+Ubuntu

All in all this is a great submission. I deployed 3 nodes, everything seems to be in order.

Thank you for a great submission! I look forward to seeing future contributions.

review: Approve

« Back to merge proposal