Charm Needed: Nginx

Hi Brandon. Thanks for submitting another full-featured charm. :)

Blockers:

* No maintainer -

$ charm proof
E: Charms need a maintainer (See RFC2822) - Name <email>

* NFS relation does nothing

I: relation nfs has no hooks

* readme doesn't explain how to actually use the charm

I really don't know what to do with this charm. Is it just for forking into other charms? I'm not sure what this charm gives me beyond 'apt-get install nginx'. Please explain this in readme.

Style questions:

* Using php, but then not using php?

run('mkdir -p /mnt/ramdisk/phpfpm-cache');
run('mkdir -p /mnt/ramdisk/tmp-cache');
run('mkdir -p /mnt/www');

Why are you using 'run' instead of just the 'mkdir' function?

* Wrong word in template

## placed by juju, do not edit or you will loose your changes

I suspect that should be 'will lose your changes'

* Using templates, but with static only values

$options = array('webroot');
$values = array('/mnt/www');
$default_site = template::replace($options,$values,template::read('default'));
template::write('/etc/nginx/sites-enabled/default',$default_site);

This seems.. fairly confusing. Whats the idea there?

* Using a non standard templating language

I'm not sure we should allow full implementations of templating in individual charms... heredocs or something established would make a lot more sense.

Setting to Incomplete until issues are addressed.

Yea , more than fair to question 99% of that , as I knew this was not a complete charm as said before but needed a break from it a few days and an initial review to kick me back off....

ok just wanted to address a couple of points tho before I started jumping back in it, really minor just clarifications to your questions ( minus obvious omissions like maintainer.

Two main points I want to address as they will likely stay in the final charm unless more discussion is had and even then .... anyhow ok ....

>Style questions:
>* Using php, but then not using php?
>run('mkdir -p /mnt/ramdisk/phpfpm-cache');
>run('mkdir -p /mnt/ramdisk/tmp-cache');
>run('mkdir -p /mnt/www');
>Why are you using 'run' instead of just the 'mkdir' function?

I did it like this very deliberately and there is actually a 3rd pattern that you might have overlooked / not added together ( un-documented , but will be when done ) the reasoning of using backtic's as well to run some commands that again may on the surface seem lke using/not using but here is why and its consistent ... php "holds" the output when shelled out and needs to be examined/echo's back out to end up in the juju log, so if you'll notice any commands that may produce output are wrapped in a simple run function that runs the command then echo's as needed ( and can have some additional error checking added as well for exit codes later ) , thus that covers using the run() vs's built-ins for those things, then the backticks are used consistently where return data is NOT expected to goto juju-log and is fed directly to a variable for immediate use ...

ok now for the "templates" if you'll notice those are also very very simple wrapper functions for what equates to a sed replace ( using perl regex instead ) just like many if not every other charm out there, the only difference is i've taken the default config and for values i KNOW i need to replace over and over I put a token in place that is easily picked up my a very simple regex or sed to keep complexity to a minimum. if this charm where in bash i would do identically the same thing ... and as far as complexity I've seen far larger and more complex functions in quite a few charms already so i dont see what keeping them out of it does , other than adding to the workload NOW of putting it into a ppa package then installing it, when yes thats eventually what will happen for the bits that prove themselves but to be honest i dont think the cost / reward is worth doing it at this point for the reasons just given , not to say that it wont ever but working > perfect , especially when every single charm will need to be basically redone for some reason or another more than one time in the next 12 months.

sooooo overall yes i 100% agree with the very small thinks mentioned above ( even if long winded , still simple from both sides )
/me goes to make the changes ...

Not available on https://jujucharms.com/