Comment 4 for bug 940140

Hi Khairul,

I had a look at your charm for an initial review and found a few blockers:

--
- PEAR

You're using PEAR to discover an address, then install symfony from PEAR. The problem with this, PEAR doesn't provide any cryptographic signature checking, or secure connections. Because of this the charm would be blocked from entering the Charm Store. To remedy this you have a few options. One would be to grab the URL for the latest download, get a valid and verified hash from it, then use charm tools and ch_get_file to download the latest archive and verify the payload. An alternative option would be to download the PEAR module and include it in the charm, then have PEAR install it locally. This way you can continue using most of your hooks without much modification.

- db-relation-changed

This hook appears to have the meat of it commented out. By the looks of it, the hook doesn't appear to do anything but restart apache and open port. This is also the second place you open ports. If a charm is un-usable without a relation the port shouldn't be opened until that relation is satisfied. So in the case of this charm, if Symfony can't operate without a database the port should open in db-relation-changed and not in the install.

- website-relation-joined

Currently doesn't do anything, relations should provide something, the website/http interface should provide a port and hostname[1]

-install

You restart apache during the install, but "start" Apache in the start hook. the start hook will "fail" since starting apache when it's already running will error out. Either remove the restart in the install hook, or make apache restart in the start hook. In my opinion the latter would be preferred.

-- Non-critical nit-picks

From the charm proof:

W: summary sould be less than 72
W: no README file

- Summary should be less than 72
 I think you can safely ignore this warning, but just wanted to bring it to your attention. Currently your summary is 81 characters.

- no README file
It's nice to include a readme file to show example uses and getting started commands for the charm.[2]

---

Overall great first cut! I look forward to using and seeing this completed soon :D

[1]: https://juju.ubuntu.com/Interfaces/http
[2]: http://charms.kapilt.com/charms/oneiric/limesurvey