new charm squid-forwardproxy

[Alexander List]

bzr+ssh://bazaar.launchpad.net/~alexlist/charms/precise/squid-forwardproxy/trunk/

[Juan L. Negron (negronjl)]

Hi Alexander:

Thank you for the charm. Her is some feedback:

Running charm proof on the charm returns:
E: revision file in root of charm is required

A revision file is required for all charms. This is how we keep track of charms for upgrades and new deployments.

The README file should have an overview of the charm as well as instruction on how to deploy it, any configuration values that can be changed as well as how to access the charm once deployed.

I'd be happy to review the charm again once the issues above have been addressed.

Thanks you for the submission ... It's almost there.

Thanks,

Juan

[Alexander List (alexlist)]

Hi Juan,

done as requested.

WRT accessing the charm once deployed, I'd need some example README. There are charms in the charm store that don't contain a README at all or that don't contain this information, e.g. distcc, tomcat7.

In the meantime, I mentioned the required relations from metadata.yaml.

I also ask myself how much sense it makes to duplicate / c&p information from the description and config.yaml/metadata.yaml to the README ...

Thanks for your help reviewing this charm!

[Alexander List (alexlist)]

Changed permissions, please review.

[Jim Baker (jimbaker)]

1. I checked out your charm and ran charm proof from
top level:

~/canonical/charmq/precise/squid-forwardproxy$ charm proof
E: revision file in root of charm is required

All errors must be fixed before we can promulgate. But this error is a
simple one to fix: simply add a file named "revision" and initialize
with some integer value that will then monotonically increase. 1 is a
good choice. This file will then allow charm users - and especially
any developers who branch your charm to hack on it - to use juju
upgrade-charm.

2. We need you to be the listed maintainer. In metadata.yaml, we
have this key:

maintainer: "Matthew Wedgwood <email address hidden>"

It's OK if you're taking over from Matthew. Just need you to be listed here.

3. Don't simply copy over config.yaml into the README. Instead you
should think of how this charm can be browsed by
jujucharms.com. config.yaml and its settings are readily available
there.

4. Instead, indicate in the README how we can deploy some example
stack with your charm. So your charm provides two interfaces, http and
nrpe-external-master (for working with the nrpe subordinate). Show us
how to use juju deploy and juju add-relation to configure squid as a
forward proxy, or to work with Nagios, using say mysql/wordpress or
example of your choice. This means users can immediately take
advantage of this charm.

BTW, I recommend writing your README file in markdown. This means
using using README.md for the specific name; see
https://bazaar.launchpad.net/~charmers/charms/precise/mysql/trunk/view/head:/README.md
and how it appears http://jujucharms.com/charms/precise/mysql). This
will make it easier for you to describe how to deploy your charm in a
nice looking way.

5. You have a hook named env-dump. I guess this is to simplify
debugging with juju debug-hooks. I think it's best to keep that
separate, but that's more of a style issue.

[Juan L. Negron (negronjl)]

Hi guys:

The issues mentioned by Jim above have not been addressed ... I'd be happy to review it once that's done.

-Juan

[Alexander List (alexlist)]

- fixed .bzrignore (it contained revision, preventing that file from being
pushed) and README.md, pushed and notified charmers
- cherrypicked from personal branch, merged with canonical-marshal branch
and replace-pushed to LP again
- all requirements above met

[Juan L. Negron (negronjl)]

Reviewing this now.

-Juan

[Juan L. Negron (negronjl)]

All of the identified issues seem to have been addressed.
This looks good to me.

Approved.

-Juan

[Juan L. Negron (negronjl)]

Charm promulgated.

-Juan