new charm squid-forwardproxy

Bug #1098599 reported by Alexander List
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Juju Charms Collection
Fix Released
Undecided
Unassigned

Bug Description

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

Revision history for this message
Juan L. Negron (negronjl) wrote :

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

Changed in charms:
status: New → Incomplete
Revision history for this message
Alexander List (alexlist) wrote :

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!

Haw Loeung (hloeung)
tags: added: canonical-webops-juju
Revision history for this message
Alexander List (alexlist) wrote :

Changed permissions, please review.

Jorge Castro (jorge)
Changed in charms:
status: Incomplete → Fix Committed
Revision history for this message
Jim Baker (jimbaker) wrote :

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.

Revision history for this message
Juan L. Negron (negronjl) wrote :

Hi guys:

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

-Juan

Changed in charms:
status: Fix Committed → Incomplete
Revision history for this message
Alexander List (alexlist) wrote :

- 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

Changed in charms:
status: Incomplete → New
Revision history for this message
Juan L. Negron (negronjl) wrote :

Reviewing this now.

-Juan

Revision history for this message
Juan L. Negron (negronjl) wrote :

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

Approved.

-Juan

Changed in charms:
status: New → Fix Committed
status: Fix Committed → Fix Released
Revision history for this message
Juan L. Negron (negronjl) wrote :

Charm promulgated.

-Juan

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.