Charm needed: Symfony
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| Juju Charms Collection |
Medium
|
Khairul Aizat Kamarudzzaman |
Bug Description
Charm for Symfony
Changed in charms: | |
assignee: | nobody → Khairul Aizat Kamarudzzaman (fenris) |
Changed in charms: | |
status: | New → In Progress |
tags: | added: new-charm |
Changed in charms: | |
status: | In Progress → Confirmed |
Clint Byrum (clint-fewbar) wrote : | #2 |
Khairul, please attach your charm branch to this bug, or note the URL to it here in the comments. Thanks!
Clint,
thanks for the comment , here is my charm branch :
Marco Ceppi (marcoceppi) wrote : | #4 |
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-
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:/
[2]: http://
Changed in charms: | |
status: | Confirmed → Incomplete |
updated to the above comment accept the PEAR part .. still working on it either whan to check the downloaded package with hash md5 or include the package inside the charm , plz re-review
already push to precise trunk https:/
Changed in charms: | |
status: | Incomplete → Fix Committed |
Juan L. Negron (negronjl) wrote : | #7 |
Reviewing it now.
-Juan
Juan L. Negron (negronjl) wrote : | #8 |
This looks good to me.
-Juan
Juan L. Negron (negronjl) wrote : | #9 |
Actually ... I mis-spoke.
There should be a README file that exlplains the purpose and usage of this charm. I would ask that you please add the README file and re-submit for review. An example of a README file can be found here: http://
I also would ask that the summary stays under 72 characters.
-Juan
Changed in charms: | |
status: | Fix Committed → Incomplete |
Hi Khairul,
not officially associated with ~charmers, but very interested in symfony project and stoked to see this get picked up to be charmed. looks like you did a great job on the charm too. Are you planning on doing a charm for Symfony 2? there's some significant improvements to Symfony 2 and it's quite stable. 1.4 is considered the "legacy" branch, and while still supported, development is focused on the newer release.
if you're not interested in doing a charm for symfony 2, please comment to that effect, and i'd be glad to take it up.
cheers.
Juan,
Thanks for your comment n review ... ill fix it as soon as i arrive my home town or else in the long flight journey back ...
Nathan,
Yeah , sure ill try to do the Symfony 2 charm. Maybe u can help me on the relation changing the yml file for the database connection. I also will working on it .. but we need to have differ option for the database .....
great news :) if you want some help working on it, i'm on IRC, or feel free to shoot me an email.
Amit Rana (amit-rana) wrote : | #13 |
Hi Nathan, Khairul,
Was there any progress on symfony2 charm?
Changed in charms: | |
importance: | Undecided → Medium |
Updated command to restart apache after creating symlink in hooks/install
Need someone to retest the charm